-
Notifications
You must be signed in to change notification settings - Fork 1.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
✨ Add release notes expander functionality #10091
✨ Add release notes expander functionality #10091
Conversation
Hi @chandankumar4. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
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.
Nice work! I generated release notes for v1.6.0-rc.1
and compared it to the existing release notes here. It looks like the intention is to move everything under Kubernetes version support
into the dropdown section, and to list changes from the previous release candidate or beta version (v1.6.0-rc.0
) in this case outside of the dropdown. This is because the generated content is from v1.5.0 rather than the previous release candidate or beta version. Let me know if this makes sense!
Made the changes accordingly. Added
And generated notes will be
|
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.
I ran the following command as it was listed in the PR description and get a a panic: panic: runtime error: index out of range [1] with length 1
./bin/notes --release v1.6.0 --pre-release-version true > CHANGELOG/v1.6.0.md
I think you might need to enforce setting --previous-release-version
if --pre-release-version
is set to true.
This also seems to happen for regular releases without setting --pre-release-version true
. For example, running this gives me the same error.
RELEASE_TAG=v1.6.0 make release-notes
hack/tools/release/notes/print.go
Outdated
@@ -79,7 +81,11 @@ func (p *releaseNotesPrinter) print(entries []notesEntry) { | |||
fmt.Printf("🚨 This is a RELEASE CANDIDATE. Use it only for testing purposes. If you find any bugs, file an [issue](https://github.com/%s/issues/new).\n", p.repo) |
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 could either be a release candidate or beta release. You can either change this based on how the release tag string is formatted, or just include both RELEASE CANDIDATE/BETA RELEASE
and have the user manually change this.
a046390
to
6931f3a
Compare
Looks like there are still some errors when running the release notes tool test with
|
When running this command, I noticed that
|
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.
Thanks for making this change! I think the unit test make test-release-notes-tool
is still failing.
Also one more nit: The tool should be printing this line inside the dropdown:
https://github.com/willie-yao/cluster-api/blob/main/CHANGELOG/v1.6.0-rc.1.md#L32
Thanks for the review, made the required changes and fixed the test cases also |
ae6800e
to
69797b6
Compare
I tried running this command |
8054541
to
3b72eb8
Compare
@willie-yao sorry for the confusion, It was messed up while resolving review comment as |
No problem! Thanks for the rebase and quick fix! /lgtm |
LGTM label has been added. Git tree hash: 2242a031bb85b77ae23193527a6ba37449d4d124
|
hack/tools/release/notes/print.go
Outdated
fmt.Print(`<details> | ||
<summary>More details about the release</summary> | ||
|
||
:warning: **RELEASE CANDIDATE NOTES** :warning: |
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.
Should we change here between beta and RC releases? Or is this only for RCs anyway?
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.
It's for both, thanks for pointing out, made the changes.
hack/tools/release/notes/process.go
Outdated
var fromTag string | ||
if previousRelease.value != "" { | ||
fromTag = previousRelease.value | ||
} else { | ||
fromTag = d.fromTag | ||
} |
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.
only a nit, no merge blocker
var fromTag string | |
if previousRelease.value != "" { | |
fromTag = previousRelease.value | |
} else { | |
fromTag = d.fromTag | |
} | |
fromTag := d.fromTag | |
if previousRelease.value != "" { | |
fromTag = previousRelease.value | |
} |
Signed-off-by: chandankumar4 <[email protected]>
3b72eb8
to
18f4d01
Compare
/lgtm |
LGTM label has been added. Git tree hash: c34f71985015404545125917dda2370c603ed59d
|
/lgtm Let's start using this and eventually iterate while working on 1.7 pre releases |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/unhold |
What this PR does / why we need it:
Add expander in generated release notes
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #9319
/area release