-
Notifications
You must be signed in to change notification settings - Fork 398
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
Update release notes draft to version v1.30.0-alpha.3 #2449
Update release notes draft to version v1.30.0-alpha.3 #2449
Conversation
ca56d8b
to
65a85a0
Compare
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.
Just a few things to note regarding empty release notes: we can write a note with the help of the PR, or simply state "none" if it's empty. Other than that, all the notes looks good to me!
65a85a0
to
19d363f
Compare
19d363f
to
21498e2
Compare
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.
@npolshakova Looking good so far! Just a few more suggestions!
ad4ae1f
to
bbc7e8d
Compare
/approve |
/lgtm |
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 this PR!
Just a suggestion related to the NONE files. Please remove/delete them.
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.
Please remove this file as its release note is NONE.
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 be removed! Should we update the release-notes-draft.json
as well?
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.
Yes, remove them from the release-notes-draft.json
also.
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.
@ramrodo can you confirm?
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.
So do we want to leave the original text, and empty string or "NONE" in the json file? #2449 (comment)
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.
Do not keep the original text nor the NONE string, maybe remove it all.
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.
Do we want to remove all the NONE
release notes from the release-notes-draft.json? Or just the ones with the extra text? For example, there are PRs like 120631 already on master: https://github.com/kubernetes/sig-release/blob/master/releases/release-1.30/release-notes/release-notes-draft.json#L263
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.
@ramrodo Just want to make sure it's fine to keep 120631, 122569, 122692, 123051, 123438, 123488, 123490 in.
It looks like prior release have the NONE string: https://github.com/kubernetes/sig-release/blob/master/releases/release-1.25/release-notes/release-notes-draft.json#L517
But the weird parsing from the release notes collector looks like it's a new bug that has only happened in 1.29 and 1.30:
"text": "```\n\n#### Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:\n\n\u003c!--\nThis section can be blank if this pull request does not require a release note.\n\nWhen adding links which point to resources within git repositories, like\nKEPs or supporting documentation, please reference a specific commit and avoid\nlinking directly to the master branch. This ensures that links reference a\nspecific point in time, rather than a document that may change over time.\n\nSee here for guidance on getting permanent links to files: https://help.github.com/en/articles/getting-permanent-links-to-files\n\nPlease use the following format for linking documentation:\n- [KEP]: \u003clink\u003e\n- [Usage]: \u003clink\u003e\n- [Other doc]: \u003clink\u003e\n--\u003e", |
@@ -87,8 +180,8 @@ | |||
}, | |||
"118389": { | |||
"commit": "be77b0b82b01a3fc810118f095594ec8bdd3c3aa", | |||
"text": "```\n\n#### Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:\n\n\u003c!--\nThis section can be blank if this pull request does not require a release note.\n\nWhen adding links which point to resources within git repositories, like\nKEPs or supporting documentation, please reference a specific commit and avoid\nlinking directly to the master branch. This ensures that links reference a\nspecific point in time, rather than a document that may change over time.\n\nSee here for guidance on getting permanent links to files: https://help.github.com/en/articles/getting-permanent-links-to-files\n\nPlease use the following format for linking documentation:\n- [KEP]: \u003clink\u003e\n- [Usage]: \u003clink\u003e\n- [Other doc]: \u003clink\u003e\n--\u003e", |
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 PR had the /release-note-none
label, where is the text here coming from?
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 wonder if this happens when the release note block in the PR is left empty instead of adding "NONE" as requested in the instructions, or when they add it in the wrong place. For example: kubernetes/kubernetes#121565
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 probably a bug. We can remove this text (as you did) for now and submit an issue to kubernetes/release repo for debugging and fixing this (if it applies).
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.
bbc7e8d
to
2ee1a97
Compare
2ee1a97
to
5922704
Compare
@@ -1598,6 +1864,22 @@ | |||
], | |||
"duplicate": true | |||
}, | |||
"122569": { |
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.
@npolshakova PRs like this should be removed
5922704
to
3cc6fc8
Compare
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.
/lgtm
/assign @ramrodo |
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 think this PR is enough good for now. Let's merge this PR and we can submit another if we need to fix any missing NONE string later.
Thanks for the hard work and the patience.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: npolshakova, ramrodo, rashansmith 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 |
What type of PR is this?
/kind documentation
What this PR does / why we need it:
This PR updates the Release Notes Draft to k/k v1.30.0-alpha.3
Which issue(s) this PR fixes:
Special notes for your reviewer:
This is an automated PR generated from
krel The Kubernetes Release Toolbox