[WIP] Improve clarity/asks in PR template and request additional metadata #81278
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,64 +1,115 @@ | ||
| <!-- | ||
| Thanks for submitting a pull request! | ||
|
|
||
| If this is your first time submitting a PR, please familiarise yourself with: | ||
|
|
||
| Contributor Guidelines | ||
| https://git.k8s.io/community/contributors/guide#your-first-contribution | ||
|
|
||
| Developer Guide | ||
| https://git.k8s.io/community/contributors/devel/development.md#development-guide | ||
|
|
||
| The following are additional useful links for PRs: | ||
|
|
||
| Best Practices for Faster Reviews | ||
| https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews | ||
|
|
||
| Marking Unfinished PRs for Review or as a Work in Progress (WIP) | ||
| https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests | ||
|
|
||
| Has consideration been given to a test plan related to this PR? If appropriate, were tests added/modified to cover this PR? You can find more information in: | ||
|
|
||
| Testing Guide | ||
| https://git.k8s.io/community/contributors/devel/sig-testing/testing.md | ||
| --> | ||
|
|
||
| **Which is the sponsoring or primary SIG associated with this PR?** | ||
| <!-- | ||
| Label the sponsoring or primary SIG associated with this PR. | ||
|
|
||
| For reference on available SIGs, you can find more details at: | ||
| https://github.com/kubernetes/test-infra/blob/master/label_sync/labels.md#labels-that-apply-to-all-repos-for-both-issues-and-prs | ||
|
|
||
| Add only ONE sig here via a /sig-primary <name> entry. | ||
| --> | ||
| /sig-primary | ||
|
|
||
| **What type of PR is this?** | ||
|
There was a problem hiding this comment. I would prefer to keep the "uncomment" format since it's easier than typing it out completely. There was a problem hiding this comment. I'll revert and add the "uncomment" text back in. Can a PR specify multiple "kinds"? Does this make sense. If so, should I remove the language around only selecting one? There was a problem hiding this comment.
Generally, most PRs have a single "kind" but it is possible to have multiple "kinds" too -- a PR can have
No strong opinions on this one. I think it should be fine to remove the language around selecting one, but a thing to note here is that we need at least one "kind" or else the PR will get the There was a problem hiding this comment. @nikhita we were primarily hoping for something to signify a deprecation. Whether a It may also be useful to have a We have changed the release notes template significantly over the past couple of releases with There was a problem hiding this comment. @nikhita The use of blockquote here and the requirement for having to hit enter and remove leading whitespace felt complicated. I wish markdown had a line comment character that we could use here. I have updated the PR by using the |
||
| <!-- | ||
| Label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. | ||
|
|
||
| For reference on required PR labels, you can find more details at: | ||
| https://git.k8s.io/community/contributors/devel/sig-release/release.md#issuepr-kind-label | ||
|
|
||
| Uncomment at least one of the following /kind entries that are relevant to this PR. You can do this by removing the strikethrough characters `~~` wrapping the entries you want associated with this PR. | ||
| --> | ||
| ~~/kind api-change~~ | ||
| ~~/kind bug~~ | ||
| ~~/kind cleanup~~ | ||
| ~~/kind design~~ | ||
| ~~/kind documentation~~ | ||
| ~~/kind failing-test~~ | ||
| ~~/kind feature~~ | ||
| ~~/kind flake~~ | ||
|
|
||
| **What this PR does / why we need it**: | ||
|
|
||
|
|
||
| **Which issue(s) this PR fixes**: | ||
| <!-- | ||
| Add references to issues that are fixed by this PR by using one or more of the following templates: | ||
|
|
||
| Fixes #<issue number> | ||
| Fixes <issue url> | ||
|
|
||
| NOTE: The linked issue will automatically be closed when the PR is merged. | ||
|
|
||
| If the PR is about failing-tests or flakes, please post the related issues/tests in a comment and do not use the Fixes templates above. | ||
| --> | ||
| Fixes # | ||
|
|
||
| **Special notes for your reviewer**: | ||
|
|
||
|
|
||
| **What user facing changes are introduced by this PR?**: | ||
| <!-- | ||
| Indicate whether there are any user facing changes that should be added to the Release Notes. | ||
|
|
||
| If there are no user facing changes, write "None" text in the release-note block below. If your PR does introduce user facing changes, then a release note is required and should be added in the release-note block below. | ||
|
|
||
| If the PR requires additional action from users switching to the new release, start the release-notes block with the text "Action required". | ||
|
|
||
| Familiarise yourself with the instructions for writing a release note: | ||
| https://git.k8s.io/community/contributors/guide/release-notes.md | ||
| --> | ||
| ```release-note | ||
|
|
||
| ``` | ||
|
|
||
| **Should this PR be associated with a deprecation notice?** | ||
| <!-- | ||
| Indicate whether this PR implements a change that requires users to be informed of a deprecation of behaviour. | ||
|
|
||
| If your PR does require a deprecation note, then remove the `None` from the deprecation-note block below and add your notes. | ||
| --> | ||
| ```deprecation-note | ||
| None | ||
|
There was a problem hiding this comment. I'd prefer not to default this because we want people to consciously apply the release note. An empty block will lead to There was a problem hiding this comment.
Very much this ^^^ There was a problem hiding this comment. I added this in since there are a large number of release notes with variations of none, n/a etc. I wanted the default to help those folks. But I totally see both of your concerns now. I'm slowly building up an understanding of the inter-dependent bits here. I'll remove the default and adjust the comment appropriately. |
||
| ``` | ||
|
|
||
| **Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.**: | ||
|
There was a problem hiding this comment. I would not change this whole section again in this PR to move faster forward. |
||
| <!-- | ||
| Provide links to any additional relevant documentation in the docs block below ONLY if you have added a release note, otherwise leave it blank. | ||
|
|
||
| The Kubernetes Release Notes website (https://relnotes.k8s.io) provides additional documentation to the end users about the changes in this PR. | ||
|
|
||
| If you decide to add links which point to resources within git repositories, please ensure that the appropriate revision is included in the link and not generic ones like `master`. This ensures that links reference a specific point in time, rather than a document that may change over time. The same applies to external documentation, which may be not available any more because it got updated to a more recent version. | ||
|
|
||
| See here for guidance on getting permanent links to files: https://help.github.com/en/articles/getting-permanent-links-to-files | ||
|
|
||
| Use one or more of the following templates in the docs block for linking to documentation: | ||
| - [KEP]: <url> | ||
| - [Usage]: <url> | ||
| - [Other doc]: <url> | ||
| --> | ||
| ```docs | ||
|
|
||
|
|
||
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.
A PR can touch areas that target multiple SIGs and we already have this information with the
sig/labels, so asking to mention the SIGs here would lead to multiple sources of truth 😬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.
@paulbouwer for some additional context. Depending on what areas of the codebase are touched by the PR, label(s) will automatically be applied via theOWNERS files. For example, this PR touches content in the
.githubdirectory and k8s-ci-robot automatically applied the labelsig/contributor-experiencekubernetes/.github/OWNERS
Lines 15 to 16 in f2c82f4
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.
+1, this is unneeded.
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.
@nikhita - this was specifically targeting the primary sig based on the request in issue #668. @onyiny-ang - it may be worth discussing this in more detail. Looking at the release-notes-draft.md, I'm not sure I can see the reason for the primary sig collection.
@mrbobbytables - I have been wondering how that aspect of the label magic worked. Thanks 👍
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.
Sorry in advance because this will be a bit long winded, but from the release notes standpoint, we were hoping to collect the singular SIG that is primarily responsible for the change the PR brings so that when we are sorting release notes (or ideally programming the release notes tool to collect release notes for a release) we are able to easily identify which SIG the note should be filed under. Having multiple SIG labels helps identify all areas that a PR may impact or contribute changes to and the release notes website makes this easier for users to digest.
However, for the release notes draft, the relnotes tool sorts each note that is tagged with multiple SIG labels to buckets created for that note (ie. there will be a SIG API Machinery bucket for PRs with only that SIG label as well as a bucket for all notes labelled with both SIG API Machinery and SIG Cluster Lifecycle) and this makes things confusing for the relnotes team and contributors trying to find all of the pertinent notes associated with what their SIG has accomplished/changed for a release. For the relnotes team, which may be fairly new contributors, it can be impossible to determine which SIG to sort the PR under without reaching out to every contributor who submitted a PR tagged with multiple labels.
As we're still generating the human curated notes, it would greatly help the relnotes team and contributors who have to look through the release notes draft if we were able to identify the SIG that the release note should be filed under automatically. This was what I had in mind when I was requesting the "primary-sig" field to be added. BUT I'm certainly open to better ideas!
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 seems like this is a non-issue for the release notes website. It is just the markdown release notes draft where this is an issue. And given how many of the current release notes fall into multiple sigs, I can see how this can create a lot of manual work.