Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

doc: change requirements for objection dismissal #35037

Closed
wants to merge 1 commit into from
Closed

doc: change requirements for objection dismissal #35037

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions doc/guides/collaborator-guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,10 @@ actionable steps alongside the objection is recommended.
If the objection is not clear to others, another collaborator can ask an
objecting collaborator to explain their objection or to provide actionable
steps to resolve the objection. **If the objector is unresponsive for seven
days after a collaborator asks for clarification, another collaborator can
dismiss the objection**.
days after a collaborator asks for clarification or proposes a
solution or compromise for the objection, another collaborator can dismiss
the objection**. Be mindful of holidays and vacations before dismissing an
objection.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important to note that -1 responses are still possible "I do not think we should do it all because of X and Z" and in those cases the TSC must get involved as no consensus can be reached.

As it is currently changed in this PR it leaves "landing by exhaustion" possible by asking for feedbacks until the objector do not reply anymore.

Note that this is very theoretical and something that would likely be escalated by the TSC, but it can be very frustrating if the objector has less time than the author of the PR.

I think we should add (or similar):

An objector has the right to mark their objection as final, in which case the PR can be escalated to the TSC by adding the tsc-agenda label.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important to note that -1 responses are still possible "I do not think we should do it all because of X and Z"

Just to make sure I understand what you mean, the "-1 responses are still possible" are on top of the objector using the "Requested changes" feature, correct? It would be something like:

  1. Objector Request Changes
  2. PR author tries to reach consensus
  3. Objector says "no, -1 I don't think this should ever land"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is currently changed in this PR it leaves "landing by exhaustion" possible by asking for feedbacks until the objector do not reply anymore.

Right, but without it our policy leaves a "blocking by exhaustion" hole, the blocker can just block and never engage again, which IMO goes against collaboration principles.

Note that this is very theoretical and something that would likely be escalated by the TSC, but it can be very frustrating if the objector has less time than the author of the PR.

Even with the "final objection", if the objector wants their objection to be final but they are not involved for a week the objection would be dismissable (if the didn't "mark it as final" when requesting changes). Furthermore, I would like to avoid a new "objection level" which is based on comment, since the reason we changed our objections policy was to avoid "comment based objections", which can easily lead to misunderstandings.

So what about we go with @guybedford suggestion: "If the objector is not responsive for seven days after a collaborator asks for clarification or proposes a solution or compromise for the objection, a TSC member who is not involved in the PR can decide if the objection should be dismissed or if the PR should be escalated.". Honestly, I don't like this one either, it doesn't really solve the issue where an objector meant for the objection to be final (but didn't say so explicitly or explicitly enough) and it might lead to PRs always waiting seven days before escalating to TSC decision.

We could remove the dismissal by collaborators altogether, but that would require the TSC to be more hands-on on PRs with objections, and we need to ensure the TSC can reach decisions effectively wrt objections asynchronously, outside the meeting. We could include all PRs with objections in the tsc-agenda for awareness (different from agenda items) and/or send a daily summary with new PR objections/weekly summary with all objections to the TSC mailing list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The case that I would like to see handled:

  1. objector asks to see XYZ changed
  2. OP change XYZ
  3. OP dismiss the objection after 7 days because the objector did not engage further

This change fixes this and it will unblock a lot of PRs. However, that's not the only thing that it changes.

I do not want to transform the dismissing of reviews to become a vehicle of abuse. You are asking that if somebody wants to keep their opinion heard to never take more than one week off from Open Source. This creates an enormous stress (at least for me), and it can lead to burnout.

Right, but without it our policy leaves a "blocking by exhaustion" hole, the blocker can just block and never engage again, which IMO goes against collaboration principles.

We are not in agreement. A key responsibility of the TSC is to solve all those situations where consensus seeking failed. We have had a few occasions where no consensus was possible. The TSC must became quicker to vote on things when those cases happen.
By our charter, we follow a Consensus Seeking model, however this PR seems to be removing the part where a vote is held if no consensus can be reached.

We can put it on the final objector to ping the TSC and tag it with tsc-agenda, motivating why their decision is final and ask the tsc to either dismiss their review or close the PR. We can have a standard phrase for that to copy and paste.
Adding the following would resolve my issue:

If it is clear that no consensus can be reached, the objector must:

1. Clearly state the reasons for his final opinion
2. Tag the PR with the `tsc-agenda` label
3. Mention @nodejs/tsc in the comment, asking them to decide to either dismiss their review or close the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are all great feedbacks, thanks, I'll try to rewrite this PR with them in mind.

The case that I would like to see handled:

  1. objector asks to see XYZ changed
  2. OP change XYZ
  3. OP dismiss the objection after 7 days because the objector did not engage further

This change fixes this and it will unblock a lot of PRs. However, that's not the only thing that it changes.

Agreed, I'll update this with this instead of a more broad dismissal rule.

I do not want to transform the dismissing of reviews to become a vehicle of abuse. You are asking that if somebody wants to keep their opinion heard to never take more than one week off from Open Source. This creates an enormous stress (at least for me), and it can lead to burnout.

I think we agree on that too, I definitely don't want to see dismissals or objections becoming a vehicle of abuse.

A key responsibility of the TSC is to solve all those situations where consensus seeking failed. We have had a few occasions where no consensus was possible. The TSC must became quicker to vote on things when those cases happen.

While I agree, I think we as TSC need to get better at this. We have 51 PRs with objections (not taking into account -1s which were valid objections for older PRs), and sometimes I see PRs not being escalated because the author is just exhausted. Furthermore, we have (or had?) a loophole where the TSC can decide to step aside and not reach a decision (which I think only happened twice, if I remember correctly). Might be worth opening a separate issue for us to discuss this further, or include it as part of the "next 10" initiative.

We can put it on the final objector to ping the TSC and tag it with tsc-agenda, motivating why their decision is final and ask the tsc to either dismiss their review or close the PR.

That doesn't solve the issue you raised where the objector has to stay somewhat active on the PR. Maybe we should rephrase it so that collaborators are more encouraged to escalate objections to the TSC instead when consensus reaching fails.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't solve the issue you raised where the objector has to stay somewhat active on the PR. Maybe we should rephrase it so that collaborators are more encouraged to escalate objections to the TSC instead when consensus reaching fails.

I think that's fair. We are asking the objector to say "I had enough, this is TSC material". We should also extend this to the OP, as they can be exhausted as well. We do not want exhausted collaborators. This will help in preventing exhaustion.


**Pull requests with outstanding objections must remain open until all
objections are satisfied**. If reaching consensus is not possible, a
Expand Down