-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Suggested edits for identity doc updates #29339
base: main
Are you sure you want to change the base?
Conversation
CI Results: |
Build Results: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the list of tempalte varaible - it's the four that refer to entity or group name that matter.
website/content/docs/upgrading/deduplication/acl-policy-templates.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/acl-policy-templates.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/acl-policy-templates.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/acl-policy-templates.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/acl-policy-templates.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/different-case.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/different-case.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/acl-policy-templates.mdx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking awesome Sarah. I think the entity and groups and external refs are still my originals so I didn't review them further.
I think the rest of this is super close and pretty much ready to merge to the final PR. There are a couple of smaller notes inline that might be worth fixing up here before we move over so we don't loose track.
website/content/docs/upgrading/deduplication/different-case.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/different-case.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/different-case.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/different-case.mdx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great Sarah! Thanks so much for your hard work on this!
I had a couple of minor suggestions inline but not blocking I don't think.
We talked about merging this back in to my PR but honestly I think at this stage the original feedback there is likely all not super relevant so how about getting one other person on Eng to review this one and then merge directly from here?
website/content/docs/upgrading/deduplication/terraform-refs.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/upgrading/deduplication/terraform-refs.mdx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These comments bring across all the previous feedback on the old PR that still applies. I think they are good ideas but not show-stoppers so see what you think @schavis
Given this being brought across, I don't see any real reason to merge this back to the other PR vs. just merge directly!
SGTM. I'll work through the remaining comments and merge the docs today, but feel free to ping me (or create a new PR) if I miss anything :) |
Co-authored-by: Paul Banks <[email protected]>
The base branch was changed.
Description
What does this PR do?
TODO only if you're a HashiCorp employee
backport/
label that matches the desired release branch. Note that in the CE repo, the latest release branch will look likebackport/x.x.x
, but older release branches will bebackport/ent/x.x.x+ent
.of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
in the PR description, commit message, or branch name.
description. Also, make sure the changelog is in this PR, not in your ENT PR.