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.

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

doc: change requirements for objection dismissal #35037

Closed
wants to merge 1 commit into from

Conversation

mmarchini
Copy link
Contributor

Previous version would allow dismissal only if the objector is
unresponsive to clarifications, but not if they are unresponsive while
other collaborators are trying to reach consensus with them. In the
spirit of collaboration the objector needs to be open to discuss
possible alternatives or should be willing to convince the PR author to
drop it. Only if reaching consensus is not possible (either to close the
PR or to make changes to it) the issue should be escalated to TSC.

Therefore the guideline is changed to allow dismissal if the objector is
unresponsive in the face of "a collaborator proposing a solution or a
compromise for the objection". Also adds a note for collaborators to be
mindful of holidays and vacations before dismissing an objection.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Previous version would allow dismissal only if the objector is
unresponsive to clarifications, but not if they are unresponsive while
other collaborators are trying to reach consensus with them. In the
spirit of collaboration the objector needs to be open to discuss
possible alternatives or should be willing to convince the PR author to
drop it. Only if reaching consensus is not possible (either to close the
PR or to make changes to it) the issue should be escalated to TSC.

Therefore the guideline is changed to allow dismissal if the objector is
unresponsive in the face of "a collaborator proposing a solution or a
compromise for the objection". Also adds a note for collaborators to be
mindful of holidays and vacations before dismissing an objection.
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Sep 3, 2020
@guybedford
Copy link
Contributor

To be honest, I'm not sure what value the rewording really has here since as discussed previously intent matters first and foremost here so if clarifying these kinds of details becomes important it is almost to the detriment of intent in some ways.

I'm still wondering if it might make sense for objection dismissal to be a TSC-only process. My specific concern is if this is the only way to get around a block, then escalating an argument to the point of communication breakdown, then dismissing the objection because communication has broken down, becomes a constructive path forward to remove blocks.

@guybedford
Copy link
Contributor

For example, it could be possible to word that the dismissal should be done by a person from the TSC, but retain the same process.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@Trott
Copy link
Member

Trott commented Sep 5, 2020

@guybedford Can you clarify if you're -1 on this (and so it shouldn't land) or just kind somewhere around +/-0.0 (where you're not sure it has value and you think other changes that can happen now or later might be better, but eh, whatever, it can land anyway)? I'm not sure if I should/can land this at this time or not. ("-1 for the moment so we can talk more about stuff, but not ultimately -1 if there's no movement after some back-and-forth" is a totally legitimate answer IMO.)

@mmarchini
Copy link
Contributor Author

I'm still wondering if it might make sense for objection dismissal to be a TSC-only process

This PR goes on the opposite direction on purpose. As it's written today, dismissal is only possible if the objector doesn't engage after a request for clarification, not if the objector doesn't engage to reach consensus, which goes against the intent of consensus seeking described prior in the text.

My specific concern is if this is the only way to get around a block, then escalating an argument to the point of communication breakdown, then dismissing the objection because communication has broken down, becomes a constructive path forward to remove blocks.

I found this phrasing confusing, so forgive me if I misunderstood, but the intention of dismissal is not "to get around a block because communication broke down", but to allow for dismissals in cases where the objector disengages. Again, the text explicitly stated that the objector must be willing to work with the PR author on reaching consensus. Avoiding TSC process in this case makes sense IMO.

For example, it could be possible to word that the dismissal should be done by a person from the TSC, but retain the same process.

That might be reasonable, but it's a different change, I wouldn't want to mix it in this PR.

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

I propose a tiny change to better explain our process.

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.

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.

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. 

@Trott Trott added the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Sep 11, 2020
@mmarchini mmarchini removed the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Sep 24, 2020
@mmarchini
Copy link
Contributor Author

Closing as the proposal in the current state can be detrimental to our process for the reasons raised here by @guybedford and @mcollina + discussion in TSC meetings + some discussions in private. Thank you everyone who gave feedback, I'll take those into consideration when bringing more proposals to improve our consensus seeking process 💜

@mmarchini mmarchini closed this Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants