Skip to content
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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

beagins
Copy link

@beagins beagins commented Dec 18, 2024

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):
Screenshot 2024-12-19 at 11 21 24 AM

"Getting started with policies" link points to new url:
Screenshot 2024-12-19 at 11 18 42 AM

TODO only if you're a HashiCorp employee

  • Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
    • LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
    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.
  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.
  • RFC: If this change has an associated RFC, please link it in the description.
  • ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

Copy link

hashicorp-cla-app bot commented Dec 18, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

CLA assistant check

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.

@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Dec 18, 2024
Copy link

github-actions bot commented Dec 18, 2024

CI Results:
All Go tests succeeded! ✅

@beagins beagins added this to the 1.18.3 milestone Dec 18, 2024
@beagins beagins force-pushed the ui/VAULT-33146/update-link branch from 64268b6 to 3ae8075 Compare December 18, 2024 22:34
@beagins beagins marked this pull request as ready for review December 18, 2024 23:17
@beagins beagins requested review from a team as code owners December 18, 2024 23:17
@beagins beagins requested a review from Monkeychip December 18, 2024 23:17
Copy link

Build Results:
All builds succeeded! ✅

anwittin
anwittin previously approved these changes Dec 18, 2024
Copy link
Collaborator

@anwittin anwittin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

Copy link
Contributor

@hellobontempo hellobontempo Dec 18, 2024

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

Copy link
Author

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 !!

Copy link
Contributor

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 😄

Copy link
Contributor

@hellobontempo hellobontempo left a 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

@beagins beagins modified the milestones: 1.18.3, 1.18.4 Dec 18, 2024
@beagins
Copy link
Author

beagins commented Dec 19, 2024

Ran yarn test:ent against latest vault-enterprise server:

1..202
# tests 202
# pass  192
# skip  10
# todo  0
# fail  0

# ok

Copy link
Contributor

@hellobontempo hellobontempo left a 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!! 🎉 🎉 🎉

hellobontempo
hellobontempo previously approved these changes Dec 19, 2024
@beagins beagins changed the title [VAULT-33146] Update vault tutorial link [VAULT-33146] Update tutorial link for creating a policy Dec 19, 2024
@beagins beagins removed this from the 1.18.4 milestone Dec 19, 2024
@beagins beagins added this to the 1.19.0-rc milestone Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-changelog ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants