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

Meta: methods of objection #34564

Closed
devsnek opened this issue Jul 30, 2020 · 10 comments
Closed

Meta: methods of objection #34564

devsnek opened this issue Jul 30, 2020 · 10 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@devsnek
Copy link
Member

devsnek commented Jul 30, 2020

The collaborator guild mentions that collaborators may object, but does not define the method of the objection. I assume it is unspecified on purpose, but it may be helpful to codify it to avoid situations like #34562. For example, we might require objection to come with github's "request changes" feature.

@devsnek devsnek added the meta Issues and PRs related to the general management of the project. label Jul 30, 2020
@mmarchini
Copy link
Contributor

I think we avoided the Request Changes feature because of the "aggressiveness" subtext that can be implied from it. If that's still a concern, maybe we could use a label (although if there are multiple objections this could still be an issue when only one objection is withdrawn). I think I saw more widespread use of the "Request Changes" feature lately in the repo, so it might be ok for us to recommend using it from now on.

@guybedford
Copy link
Contributor

In the case in #34467 (comment) I took "I am not a fan of this" as an indication of preference and opinion and not as a formal objection. It seems that was the underlying misunderstanding in the choice of wording being misinterpreted but agreed it is better to give the benefit of the doubt in such cases.

Personally I would prefer that objections require some formality to them to avoid confusions like this in future.

@jasnell
Copy link
Member

jasnell commented Jul 30, 2020

Fortunately, GitHub has made changes to the "Request Changes" flow that make it significantly less aggressive and less scary. I don't think use of a label is going to more any more effective than that workflow, and I'd prefer that we rely on it.

The larger issue with objections that I've seen are blocking objections from one or two individuals who do not follow up with any attempt at resolving the block. These generally fall into the category, "My opinion is.... therefor I'm blocking this" without any other rationale or effort. Fortunately, these are rare, but they can be extremely annoying when there is zero engagement or room for compromise on the part of the person blocking.

@mmarchini
Copy link
Contributor

The larger issue with objections that I've seen are blocking objections from one or two individuals two do not follow up with any attempt at resolving the block. These generally fall into the category, "My opinion is.... therefor I'm blocking this" without any other rationale or effort.

No follow up is grounds to dismiss the objection per our current policy, although we don't stipulate how long until the objection can be dismissed (and we rarely dismiss objections anyway).

I'm happy adding "Request Changes" as a requirement (or strong recommendation) for collaborators to explicitly object to something. ncu even checks for objections that way, which is quite useful to avoid accidentally landing objected PRs.

"My opinion is.... therefor I'm blocking this" without any other rationale or effort. Fortunately, these are rare, but they can be extremely annoying when there is zero engagement or room for compromise on the part of the person blocking.

We might want to elaborate more how collaborators should object, making it clear that objections should not be based solely on personal preference and the objector must be willing to work collaboratively in a non-obstructive or combative manner to reach consensus or find a different solution.

@Trott
Copy link
Member

Trott commented Aug 1, 2020

I'm happy adding "Request Changes" as a requirement (or strong recommendation) for collaborators to explicitly object to something.

I think that's exactly the thing to do here.

@guybedford
Copy link
Contributor

I would really value us making such a requirement. There shouldn't have to be any doubt as to the existence or nature of an objection, and ensuring things are always completely clear seems like it would aid collaboration.

@joyeecheung
Copy link
Member

I usually only use "Request Changes" when I actually want to block it (just like some people also do, I believe), and I try to leave some comments about what I'd like to see before it can be dismissed (which is me trying to imply that "if for whatever reason I fail to come back when what I would like to see is done, please dismiss it for me"). If I don't use that button it means that my comments are non-blocking. I think it would be nice to make that formality a thing (so that node-core-utils can check it).

@mmarchini
Copy link
Contributor

I think it would be nice to make that formality a thing (so that node-core-utils can check it).

I think node-core-utils already does that. If not, we can add a check for it.

@mmarchini
Copy link
Contributor

Just confirmed, ncu detects requested changes and will add a ❌ in that case:

image

mmarchini added a commit to mmarchini/node that referenced this issue Aug 5, 2020
Harden policy around objections to avoid misunderstanding and to
encourage collaboration between pull request authors and objectors.

Fixes: nodejs#34564
@mmarchini
Copy link
Contributor

#34639

MylesBorins pushed a commit that referenced this issue Aug 17, 2020
Harden policy around objections to avoid misunderstanding and to
encourage collaboration between pull request authors and objectors.

Fixes: #34564

PR-URL: #34639
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this issue Aug 20, 2020
Harden policy around objections to avoid misunderstanding and to
encourage collaboration between pull request authors and objectors.

Fixes: #34564

PR-URL: #34639
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
addaleax pushed a commit that referenced this issue Sep 22, 2020
Harden policy around objections to avoid misunderstanding and to
encourage collaboration between pull request authors and objectors.

Fixes: #34564

PR-URL: #34639
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
addaleax pushed a commit that referenced this issue Sep 22, 2020
Harden policy around objections to avoid misunderstanding and to
encourage collaboration between pull request authors and objectors.

Fixes: #34564

PR-URL: #34639
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants