Add pull request process to CONTRIBUTING docs #892
Conversation
mcwumbly
requested review from
vuil,
ankeesler,
anujc25 and
benjaminapetersen
Once a pull request is opened and the review process begins, it is not clear what is needed in order to get the pull request merged. At a high level, we want to try to make the following clear for contributors: - What is the next step towards getting this PR merged? - What's the status of that next step? - Is there anything I can/should do to help make that happen? In doing so, the process aims to clarify the following: - Who is responsible for _reviewing_ my pull requests? - Who is responsible for _approving_ pull requests? - Who is responsible for _merging_ pull requests? - What should I do if it seems like my pull request has gotten "lost in the mix"? How should I determine whether any further action is needed on my part? This is a starting point for documenting a process that we can follow and then further improve. Related: vmware-tanzu#844
mcwumbly
force-pushed
the
add-pull-request-process-docs
branch
from
acbaf37
to
5a911c6
Compare
CONTRIBUTING.md
Outdated
| 1. Review is automatically requested from CODEOWNERS and `needs-triage` label is automatically added. | ||
| 2. Assignee is added to pull request to ensure it gets proper attention throughout the process. | ||
| Typically one of the CODEOWNERS will assign themselves, but they may choose to delegate to someone else. | ||
| 3. Triage removes `needs-triage` and adds `ok-to-merge` if the pull request is generally aligned with product goals and does not conflict with current milestones; otherwise they may add a comment and a `do-not-merge/*` label. |
There was a problem hiding this comment.
I had to read this line a couple of times to make sure I knew who Triage was. I assume, based on discussion in SIG meetings and context clues, that Triage is referring to a group of tanzu-framework product owners. Do you think there is any chance we need to be more specific about who Triage is so that community members know exactly who is on the hook for this step?
There was a problem hiding this comment.
@ankeesler I think it could be some combination of: 1) the assignee, 2) tanzu-framework-reviewers, or 3) a person wearing the "triage" hat during some periodic triage meeting... without more context on how that is evolving, I was weary of being overly specific here.
In the meantime, my hope is that this final paragraph offers enough clarity on what to do if things are not progressing:
If any of the above is unclear, and there has been no new activity for 2-3 days,
the contributor is encouraged to seek further information by commenting and
mentioning the assignee or @vmware-tanzu/tanzu-framework-reviewers if there is
no assignee or they themselves are unresponsive.
cc @vuil
There was a problem hiding this comment.
Yeah we can iterate and provider more clarifications when the triage process is more nailed down. For now, I think the quoted paragraph is a useful catch-all on what actions to take when things are not progressing.
CONTRIBUTING.md
Outdated
| 1. Review is automatically requested from CODEOWNERS and `needs-triage` label is automatically added. | ||
| 2. Assignee is added to pull request to ensure it gets proper attention throughout the process. | ||
| Typically one of the CODEOWNERS will assign themselves, but they may choose to delegate to someone else. | ||
| 3. Triage removes `needs-triage` and adds `ok-to-merge` if the pull request is generally aligned with product goals and does not conflict with current milestones; otherwise they may add a comment and a `do-not-merge/*` label. |
There was a problem hiding this comment.
Yeah we can iterate and provider more clarifications when the triage process is more nailed down. For now, I think the quoted paragraph is a useful catch-all on what actions to take when things are not progressing.
vuil
added
area/repo maintenance
ok-to-merge
This label is not used for pull requests.
Related issue: vmware-tanzu#874
What this PR does / why we need it
Once a pull request is opened and the review process begins, it is not clear what is needed in order to get the pull request merged.
At a high level, we want to try to make the following clear for contributors:
In doing so, the process aims to clarify the following:
This is a starting point for documenting a process that we can follow
and then further improve.
Which issue(s) this PR fixes
Fixes #844
Describe testing done for PR
Discussion on linked issue
Release note
(no release note needed for this one)
PR Checklist
Additional information
Special notes for your reviewer