Policy and criteria for backporting to stable release branches #5137
Conversation
Signed-off-by: David Vossel <davidvossel@gmail.com>
Signed-off-by: David Vossel <davidvossel@gmail.com>
kubevirt-bot
added
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
|
||
| - **Bug Fix Only:** The backport must be a bug fix and the bug fix must be | ||
| first merged into the master branch. The only exception is when a bug only | ||
| exists in a stable branch and does not exist in the master branch. |
There was a problem hiding this comment.
Can we be explicit about preferring that a url describing the bug/issue is included in the pr text, and preferably also in the commit message?
There was a problem hiding this comment.
I'm not sure about the commit messages. These are (most of the time) being backported automatically with a label /cherrypick [release]
If we want bugs to be added to commit messages we probably need to automate this process.
@rmohr what do you think?
There was a problem hiding this comment.
Can we be explicit about preferring that a url describing the bug/issue is included in the pr text
sure, doesn't the "Detailed Description" section in this doc cover this though?
- **Detailed Description** The backport pull request's description must either
contain a detailed explanation for what the bug fix addresses or contain a
link to such a explanation present in another PR/Issue.
preferably also in the commit message?
This will be harder to enforce. Often bug fixes are split across multiple commits. I don't think we'll be successful in requiring this, but i think it should be encouraged.
There was a problem hiding this comment.
Maybe it would make sense to just add a mention of the issue id or bug url in the release note itself? This would then automatically and directly end up in the release description? Or would that be too much?
There was a problem hiding this comment.
I'd prefer a full URL. I'd assume bug trackers of multiple vendors might be involved, so better to be explicit.
There was a problem hiding this comment.
I see neither Kubernetes nor Openstack Nova uses company-specific bug trackers. I can't tell how healthy will be for the project long term.
There was a problem hiding this comment.
Maybe I wasn't clear. I'm not suggesting KubeVirt to use private bugtrackers. I'm just saying that we can tolerate any kind of private reference tucked in the commit message.
There was a problem hiding this comment.
Isn't it good enough, like david said?
Can we be explicit about preferring that a url describing the bug/issue is included in the pr text
sure, doesn't the "Detailed Description" section in this doc cover this though?
There was a problem hiding this comment.
The main thing I care about is that backports PRs provide a clear case for why the backport is necessary. That gives us two things.
- As a reviewer/approver i can make a judgement call for whether the PR is a bug fix and what functionality it impacts.
- Anyone can retroactively trace back the commit to the PR to discover why the bug fix was introduced.
My primary goal here is to ensure that both the defense and reasoning for a backport is transparent. This avoids the cycle where a couple of approvers get together out of band and decide something must be backported without documenting any sort of hint for why the backport is necessary.
I'm probably the primary offender here. This is to protect the project from me :D
So, from my standpoint, the minimum is to have all this info in the PR description. Having full info in a commit message is nice, but like i said previously, difficult to enforce (especially with our cherry-pick plugin). I don't think it's reasonable to ask someone to cherry-pick a PR, then modify the commit messages for whatever is cherry-picked. Instead they should supplement the PR's description with more info if needed for tracking why the PR is being backported.
There was a problem hiding this comment.
Isn't it good enough, like david said?
Can we be explicit about preferring that a url describing the bug/issue is included in the pr text
sure, doesn't the "Detailed Description" section in this doc cover this though?
It is certainly good enough.
|
FYI @mazzystr |
docs/release-branch-backporting.md
Outdated
| considered. It is the reviewer's and approver's responsibility to uphold this | ||
| policy. | ||
|
|
||
| - **Bug Fix Only:** The backport must be a bug fix and the bug fix must be |
There was a problem hiding this comment.
There were a lot of discussions in the past regarding to infra-related changes: It would be very helpful for infra-work and test-stabiliziation PRs, if they can just be done. Without creating somewhere extra issues (if on github, there is no clear opinion if PRs need an issue, I would prefer not, since it is close enough together).
There was a problem hiding this comment.
There were a lot of discussions in the past regarding to infra-related changes
I'm going to explicitly call out infra changes as being welcome for backporting just to make sure there's no confusion later on.
Without creating somewhere extra issues
yep, makes sense. I think i have reflected that in the doc. I'm not of the opinion that we have to have issues created somewhere for every PR we consider backporting. I think it's enough that we have a policy of ensuring that a reviewer can accurately determine using information within the description (whether links or a text explanation) the purpose of the backport.
|
|
||
| - **Bug Fix Only:** The backport must be a bug fix and the bug fix must be | ||
| first merged into the master branch. The only exception is when a bug only | ||
| exists in a stable branch and does not exist in the master branch. |
There was a problem hiding this comment.
Isn't it good enough, like david said?
Can we be explicit about preferring that a url describing the bug/issue is included in the pr text
sure, doesn't the "Detailed Description" section in this doc cover this though?
It is certainly good enough.
Signed-off-by: David Vossel <davidvossel@gmail.com>
|
To clear up any confusion about what is eligible to be backported, I added some guidelines to the document for how we define a "bug fix". |
|
@davidvossel: The following test failed, say
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. I understand the commands that are listed here. |
|
we don't need to waste CI resources on this. i'm force merging |
This PR adds a document outlining our policy and expectations when it comes to backporting bug fixes to stable release branches. It's important that we as maintainers of the project uphold the policy outlined here so we clearly communicate what fixes are going into our release branches.