-
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
[VAULT-33146] Update tutorial link for creating a policy #29226
base: main
Are you sure you want to change the base?
Conversation
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
CI Results: |
64268b6
to
3ae8075
Compare
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.
looks good
changelog/29226.txt
Outdated
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.
Woah, you even included a changelog! 🤩 PRs like this don't necessarily need a changelog since it's a pretty small change. Always feel free to ask, but when I'm deciding whether or not to include one I usually ask myself: "Is this a change that will be helpful to surface to the user?"
It is good to make changelogs specific so I'd suggest mentioning that this was specifically for a policy tutorial link if you do decide to keep the changelog 😄 Something like:
Update tutorial link for creating a policy
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.
Ah okay cool! tbh I didn't realize the changelog was optional 😅 I saw the ❌ on the ci pipeline and added it lol
I'll go ahead and update the changelog comment to be more specific, thanks @hellobontempo !!
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.
Ah I see! You can add the lable pr/no-changelog
to skip the changelog check but it doesn't block the merge 😄
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.
If this is going to be backported, I recommend adding the backport/1.18.x
label and this will automatically open a backport PR! Feel free to DM me if you want to review backporting 😄
Also, we should update the milestone to 1.18.4
Ran
|
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.
Great work! Same note about the changelog, I think it'd be useful for the PR description and title to mention that it's specifically a policy tutorial link. (Most all of our PRs will be Vault related 😄 )
Also, screenshots can be useful to include in the description for posterity since it's not always immediately clear where the link renders.
Congrats on your first PR!! 🎉 🎉 🎉
Description
What does this PR do?
Update tutorial link for creating a policy.
Had to get a little hacky and comment out an if/else conditional in order to display empty state w/ link (default state is not empty, & unable to delete/remove default policy, so not sure when empty state is actually rendered):
"Getting started with policies" link points to new url:
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.