Add policy for maintainer takeover of stale PRs#159
Conversation
fe56f73 to
3088529
Compare
| Make sure that new tests are added for bugs in order to catch regressions and tests | ||
| with new features to exercise the new functionality that is added. | ||
|
|
||
| ## Maintainer Takeover of Stale Pull Requests |
There was a problem hiding this comment.
| ## Maintainer Takeover of Stale Pull Requests | |
| ## Fast-Tracking Pull Requests |
There was a problem hiding this comment.
I think "Fast-Tracking" changes the scope in a way that doesn't match what we discussed in the maintainer summit. The original motivation was to find a way to clear the backlog of stale PRs where authors have gone unresponsive. Fast-tracking implies potentially grabbing non-stale PRs to ship things faster, which seems like a different policy.
Maybe "stale" is too narrow and we could just drop that word should we need to cover non-stale cases down the line.
There was a problem hiding this comment.
"stuck" might be a better word than "stale". Or you could be inclusive of both "Fast-tracking and un-sticking pull requests".
There was a problem hiding this comment.
I see.. I suppose both tasks are useful, fast track and very stale reassignments. The bulk of the times I've seen a maintainer take over has been for fast tracking more than a reassign. Very stale worthwhile PRs should have an issue assigned to a maintainer for shepherding as we have done on our KEP process. Have never been a fan of bot closing stale PRs so yeah the word has connotations. +1 Stuck works.. and possibly mention that as being for fast tracking and un-sticking PRs.
| ## Maintainer Takeover of Stale Pull Requests | ||
|
|
||
| The containerd project values the effort that contributors put into pull | ||
| requests. However, life priorities change, and sometimes a contributor may |
There was a problem hiding this comment.
| requests. However, life priorities change, and sometimes a contributor may | |
| requests. |
There was a problem hiding this comment.
The intent of this was to acknowledge a contributor's work is valued which is why we should preserve authorship / to be stylistically consistent with the rest of the guide. I don't want to lose the rationale entirely but maybe we can remove some of the fluff:
We recognize that sometimes a contributor may ...
There was a problem hiding this comment.
I like a little fluff.
One persons non-responsive duration timer might not align with that of the contributor or even among the maintainers. See current bots closing out stale issues/PRs/KEPs way way too fast creating yet another maintenance issue.. that of tracking stale bot notifications to step in and refresh them. Thus why I prefer un-sticking / fast tracking over stale processing.
|
|
||
| The containerd project values the effort that contributors put into pull | ||
| requests. However, life priorities change, and sometimes a contributor may | ||
| become unresponsive before a pull request is fully ready to be merged. |
There was a problem hiding this comment.
| become unresponsive before a pull request is fully ready to be merged. |
There was a problem hiding this comment.
Could you provide more context for why you want to remove this? I think this provides context with the above line on the kinds of PRs we'd like to take over
There was a problem hiding this comment.
unresponsive could be seen by the contributor as a negative review and is subjective... To me it's more like hey we love this change so much we'd like to merge it with a follow up commit aligning with some reason for amending... They may just plain disagree with a code review ask, and thus ghosting... it could be unavailable, it could be refocused but would like to pick it back up someday. Sometimes we need to take over even if they are immediately responsive.
| requests. However, life priorities change, and sometimes a contributor may | ||
| become unresponsive before a pull request is fully ready to be merged. | ||
|
|
||
| If a pull request is nearly complete but requires some final adjustments (such |
There was a problem hiding this comment.
| If a pull request is nearly complete but requires some final adjustments (such | |
| To focus on delivering bugs/features faster, maintainers may take over (assist with) contributor PRs. If a PR is close to completion, a maintainer may first request the author make the needed diffs to the commit(s) and/or by asking the author if they need/want the assist. |
There was a problem hiding this comment.
Some concerns about this wording:
- "Take over (assist with)" -- I don't think these are the same thing. Taking over a PR because the author is unresponsive, and assisting an active author are different actions with different etiquette. Conflating them risks the policy being read as license to grab PRs from contributors who are still engaged with us
- "May first request the author make the needed diffs" -- this assumes the author is responsive, which contradicts the premise of this section. The reason we needed this policy is for authors that aren't replying
I'm also not sure I agree with the premise of this being to deliver bugs or features faster (see the above comment on "Fast Tracking"). If we want to include this then we probably need another round of review to ensure we're all on the same page.
There was a problem hiding this comment.
I'm also not sure I agree with the premise of this being to deliver bugs or features faster
Commented above, but there's value in both here. It can often be faster for a maintainer to just fix trivial issues in a PR rather than requesting the author does it (which fits the "fast tracking" bit). The unresponsive case is also important.
There was a problem hiding this comment.
nod .. value in both... and with my first read it read, to me, like it was for both cases but primarily to un-stick.
Wasn't clear in the reading that this was just about reducing the aged PR list to a manageable size. So separating the two tasks seems like a good way to go.
| become unresponsive before a pull request is fully ready to be merged. | ||
|
|
||
| If a pull request is nearly complete but requires some final adjustments (such | ||
| as addressing minor review nits or resolving merge conflicts), and the original |
There was a problem hiding this comment.
| as addressing minor review nits or resolving merge conflicts), and the original |
There was a problem hiding this comment.
These examples mirror the style of other sections in this guide and help readers understand what counts as "nearly complete"
|
|
||
| If a pull request is nearly complete but requires some final adjustments (such | ||
| as addressing minor review nits or resolving merge conflicts), and the original | ||
| author is not responding to comments, a maintainer may choose to take over the |
There was a problem hiding this comment.
| author is not responding to comments, a maintainer may choose to take over the |
There was a problem hiding this comment.
Unresponsive authors were the original premise for this proposal. Including this provides necessary context for why we would choose to take over a PR.
There was a problem hiding this comment.
author may not be able to respond or disagrees with the review.. or the notice just got lost in the mail bin. Best to always be nice to the contributor. Didn't seem to need to be said. I don't want to read, I took over this PR because you (the author) didn't respond fast enough. Prefer the wording we love your diffs and are following it up with ___ to accelerate the merge.
| If a pull request is nearly complete but requires some final adjustments (such | ||
| as addressing minor review nits or resolving merge conflicts), and the original | ||
| author is not responding to comments, a maintainer may choose to take over the | ||
| pull request to ensure the work is not lost and can be merged. |
There was a problem hiding this comment.
| pull request to ensure the work is not lost and can be merged. |
There was a problem hiding this comment.
I think this line is consistent with the narrative style of this guide and re-iterates that we value the author's contributions
| - **Modifications**: The method of applying modifications (e.g., amending | ||
| existing commits for minor fixes vs. adding follow-up commits for larger | ||
| changes) is left to the maintainer's preference on a case-by-case basis. | ||
| - **Audit Trail**: Maintainers should provide a link back to the original pull |
There was a problem hiding this comment.
not sure about adding links to a PR in the commit message...
There was a problem hiding this comment.
Could you say more about the concern here? Is it that links could be dead, or the commit message is the wrong process for traceability, or something else?
For context, @dims asked offline for a way to close the loop on the original PR which is what motivated this bullet
There was a problem hiding this comment.
is that some sort of convention? git commit -m "follow-up to #" ?
The convention Fixes #123 is the more typical well known GitHub pattern for linking to issues.
Anyhow I've always presumed PRs should link to PRs/Issues and because commits have history, keep the commit history as is or squash their commits and add your commit on top with your signed commit or by amending theirs? Don't want to loose their auth signature.
There was a problem hiding this comment.
follow up.. yes you can use PR numbers in a commit message (as is done for merge commits) .. just like for issue number tracking b20e13c
Adds a section to the contributing guide outlining the process for maintainers to take over stale pull requests. This ensures valuable contributions are not lost when an author becomes unresponsive, while preserving original author credit and providing transparency on modifications, as discussed at the 2026 containerd Maintainer Summit. Assisted-by: Antigravity Signed-off-by: Chris Henzie <chrishenzie@gmail.com>
3088529 to
3705f15
Compare
|
Received offline feedback from @dims:
Updated to include a specific line for this. |
Adds a section to the contributing guide outlining the process for maintainers to take over stale pull requests. This ensures valuable contributions are not lost when an author becomes unresponsive, while preserving original author credit and providing transparency on modifications, as discussed at the 2026 containerd Maintainer Summit.
Assisted-by: Antigravity