meta: clarify pr objection process further #59096
Conversation
Based on some recent confusion around the objection process for PRs, this commit adds some additional clarification to the collaborator guide. Specifically, it clarifies that: * Objections must be made in the PR itself * All objections are considered equal... no special additional weight is given to objections from TSC members. * When mistakes happen and a PR lands despite having an unresolved objection, any revert or fixup PR is subject to the same regular objection process, albeit with a callout that fast-tracking is possible if uncontroversial.
jasnell
added
the
meta
|
Review requested:
|
RafaelGSS
left a comment
RafaelGSS
left a comment
There was a problem hiding this comment.
I 100% agree with the first two points, but the third one doesn't make sense in my opinion. If a PR was wrongly merged due to not solving all objections, we must revert that and reach a consensus - even if the consensus would be to a TSC vote. From my perspective, the repository source code must always represent the project agreement.
Co-authored-by: Yagiz Nizipli <[email protected]>
It's not always immediately clear that a revert actually is the correct solution. A follow up PR that simply addresses the concern might be the best approach. It depends entirely on what the nature of the objection is. For instance, if it's "we shouldn't have this API", then yes, revert would be appropriate. If it's, "This needed an additional test or documentation" then a follow up PR is actually better than a revert. We should also recognize that there's exceedingly little urgency involved in these things in the typical case. Changes don't really become an issue until they are shipped in a release. Most of the time rushing to revert something is often simply unnecessary. But that's my opinion, and I thank you for yours :-) ... Let's get some more points of view from more collaborators and see where the majority consensus lies. |
|
@jasnell see my proposal at #59096 (comment). |
jasnell
added
the
tsc-agenda
|
Adding to |
Co-authored-by: Jordan Harband <[email protected]>
|
@nodejs/tsc ... just pinging for additional input on this. I'd actually prefer to see if we can get consensus on this through discussion in the PR rather than having to take it to a TSC meeting. |
guybedford
left a comment
guybedford
left a comment
There was a problem hiding this comment.
Looks great, I don't envy you taking on all this feedback, but thanks for the thoughtful iteratoin here.
|
If I'm the only blocker, and concerned person, we can proceed. |
|
Given that, there are no further blocks. Going to put this on the commit-queue. |
jasnell
added
the
commit-queue
nodejs-github-bot
added
commit-queue-failed
Commit Queue failed- Loading data for nodejs/node/pull/59096 ✔ Done loading data for nodejs/node/pull/59096 ----------------------------------- PR info ------------------------------------ Title meta: clarify pr objection process further (#59096) Author James M Snell <[email protected]> (@jasnell) Branch jasnell:jasnell/several-clarifications-to-objection-process -> nodejs:main Labels doc, meta, tsc-agenda, author ready Commits 17 - meta: clarify pr objection process further - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md - Update doc/contributing/collaborator-guide.md Committers 2 - James M Snell <[email protected]> - GitHub <[email protected]> PR-URL: https://github.com/nodejs/node/pull/59096 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Stewart X Addison <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/59096 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Stewart X Addison <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> -------------------------------------------------------------------------------- ℹ This PR was created on Wed, 16 Jul 2025 21:02:23 GMT ✔ Approvals: 11 ✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/59096#pullrequestreview-3035586518 ✔ - Filip Skokan (@panva) (TSC): https://github.com/nodejs/node/pull/59096#pullrequestreview-3043020492 ✔ - Rafael Gonzaga (@RafaelGSS) (TSC): https://github.com/nodejs/node/pull/59096#pullrequestreview-3043952088 ✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/59096#pullrequestreview-3044349789 ✔ - Moshe Atlow (@MoLow): https://github.com/nodejs/node/pull/59096#pullrequestreview-3044935528 ✔ - Ruy Adorno (@ruyadorno) (TSC): https://github.com/nodejs/node/pull/59096#pullrequestreview-3045001712 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/59096#pullrequestreview-3070778996 ✔ - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/59096#pullrequestreview-3077333410 ✔ - Darshan Sen (@RaisinTen) (TSC): https://github.com/nodejs/node/pull/59096#pullrequestreview-3077689278 ✔ - Stewart X Addison (@sxa): https://github.com/nodejs/node/pull/59096#pullrequestreview-3079341237 ✔ - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/59096#pullrequestreview-3092604879 ✔ Last GitHub CI successful ℹ Green GitHub CI is sufficient -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 59096 From https://github.com/nodejs/node * branch refs/pull/59096/merge -> FETCH_HEAD ✔ Fetched commits as 134625d76139..01ff291e624b -------------------------------------------------------------------------------- [main 2d94541b13] meta: clarify pr objection process further Author: James M Snell <[email protected]> Date: Wed Jul 16 13:55:08 2025 -0700 1 file changed, 25 insertions(+), 5 deletions(-) [main 0dce8be1da] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Wed Jul 16 14:36:02 2025 -0700 1 file changed, 1 insertion(+), 1 deletion(-) [main 7e52d0e655] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Thu Jul 17 11:13:27 2025 -0700 1 file changed, 1 insertion(+), 2 deletions(-) [main 555cdbbddc] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Thu Jul 17 11:19:02 2025 -0700 1 file changed, 3 insertions(+), 2 deletions(-) [main 282c0faa20] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Thu Jul 17 11:20:11 2025 -0700 1 file changed, 2 insertions(+), 1 deletion(-) [main 4c3e8e0687] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Thu Jul 17 15:39:00 2025 -0700 1 file changed, 2 insertions(+), 2 deletions(-) [main be258476bc] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Thu Jul 17 18:24:47 2025 -0700 1 file changed, 9 insertions(+), 11 deletions(-) [main e7b4ef6e8a] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Thu Jul 17 18:26:30 2025 -0700 1 file changed, 4 insertions(+), 4 deletions(-) [main 91ef8fb0b4] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Thu Jul 17 18:28:09 2025 -0700 1 file changed, 1 insertion(+), 1 deletion(-) [main 121267946e] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Sat Jul 19 14:02:55 2025 -0700 1 file changed, 1 insertion(+), 1 deletion(-) [main a58b50b8ba] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Sat Jul 19 14:05:51 2025 -0700 1 file changed, 3 insertions(+), 3 deletions(-) [main a9f67d0a95] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Sat Jul 19 14:07:31 2025 -0700 1 file changed, 1 insertion(+), 1 deletion(-) [main 22d709186f] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Sat Jul 19 14:10:18 2025 -0700 1 file changed, 2 insertions(+), 1 deletion(-) [main 21490cffb1] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Sat Jul 19 14:12:13 2025 -0700 1 file changed, 4 insertions(+), 3 deletions(-) [main 4efcbc23d3] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Tue Jul 22 09:00:51 2025 -0700 1 file changed, 8 insertions(+), 7 deletions(-) [main 42199d8fba] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Thu Jul 31 17:46:49 2025 -0700 1 file changed, 5 insertions(+), 9 deletions(-) [main f891a2895c] Update doc/contributing/collaborator-guide.md Author: James M Snell <[email protected]> Date: Thu Jul 31 17:47:38 2025 -0700 1 file changed, 1 insertion(+), 1 deletion(-) ✔ Patches applied There are 17 commits in the PR. Attempting autorebase. Rebasing (2/34) Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- meta: clarify pr objection process further
PR-URL: #59096
|
jasnell
added
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in 31aacfa |
jasnell
removed
the
tsc-agenda
19 participants
Based on some recent confusion around the objection process for PRs, this commit adds some additional
clarification to the collaborator guide.
Specifically, it clarifies that:
@nodejs/tsc