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

doc: harden policy around objections #34639

Closed
wants to merge 5 commits into from
Closed

doc: harden policy around objections #34639

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1331fc1
doc: harden policy around objections
mmarchini Aug 5, 2020
a6e01ee
fixup! doc: harden policy around objections
mmarchini Aug 6, 2020
fceaff2
fixup! doc: harden policy around objections
mmarchini Aug 6, 2020
9d09194
fixup! doc: harden policy around objections
mmarchini Aug 6, 2020
01d8caa
fixup! doc: harden policy around objections
mmarchini Aug 9, 2020
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
24 changes: 16 additions & 8 deletions doc/guides/collaborator-guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,22 @@ needed [approvals](#code-reviews), [CI](#testing-and-ci), and
except the [wait time](#waiting-for-approvals), please add the
[`author ready`](#author-ready-pull-requests) label.

Where there is disagreement among Collaborators, consensus should be sought if
possible. If reaching consensus is not possible, a Collaborator may escalate the
issue to the TSC.

Collaborators should not block a pull request without providing a reason.
Another Collaborator may ask an objecting Collaborator to explain their
objection. If the objector is unresponsive, another Collaborator may dismiss the
objection.
If a collaborator believes a pull request should not land as is, **the "Request
Changes" GitHub feature must be used to make the objection explicit**. An
implicit objection not using the "Request Changes" feature is not considered a
mmarchini marked this conversation as resolved.
Show resolved Hide resolved
blocker for a pull request. Pull requests with an explicit objection should
Copy link
Contributor

@guybedford guybedford Aug 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Both of these two sentences seem to be restatements of the first statement. I appreciate making sure it is clear, but perhaps just a single restatement would be fine between these?

To add another variation to the mix, another restatement could be a comprehensive statement of the positive criteria for landing a PR.

A PR may land only when both the wait time has passed and there are no explicit objections as marked through the GitHub "request changes" feature.

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'll try to rephrase it so it's less repetitive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, forgot to address these before landing. Sent another PR that should reduce repetitiveness: #34702

not land until all objections are satisfied. Collaborators should not block a
pull request without providing a reason. **Providing a set of actionable steps
Copy link
Contributor

Choose a reason for hiding this comment

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

Again it might be nice to state this in the positive:

Any PR objection must include a clear reason for that objection as well as responsiveness to further discussion towards consensus. If reaching consensus is not possible...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The suggestion drops the recommendation to provide actionable steps, but I see your point. Let me try to rephrase this too.

alongside the objection is recommended, and the objector must be willing to
work with the pull request author to reach a consensus about the direction of
the pull request**. If reaching consensus is not possible, a Collaborator may
escalate the issue to the TSC.

If the objection is not clear to others, another Collaborator may 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 may
dismiss the objection**.
Copy link
Contributor

Choose a reason for hiding this comment

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

7 days feels a little arbitrary here to me. Eg what if someone is on holiday? Also I'm not sure the criteria for unresponsive are necessarily easy to define without considering what it means to request a response? Does it perhaps make sense to delegate this process to the TSC entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

7 days feels a little arbitrary here to me

So is 48h to land a PR or 7 days before we can land a PR with only one approval :D. Honestly I don't have any preference as to how long to wait until an objection can be dismissed, so I'm open to suggestions.

Also I'm not sure the criteria for unresponsive are necessarily easy to define without considering what it means to request a response

Can you provide an example of an interaction where "requesting a response" would be ambiguous? Should we require that the objector be mentioned (@) for the time to start counting?

Does it perhaps make sense to delegate this process to the TSC entirely?

I'm against this as it just adds more bureaucracy. Collaborators who don't feel comfortable dismissing an objection can always ping TSC if they want to.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess I'm also just trying to err on the side of encouraging collaboration over process, but agreed these rules are for when collaboration breaks down and having a clear non-process-based clause for moving forward does seem like it could help in such dire circumstances! That said, it seems like it could be healthier to remove any collaborator as a collaborator entirely if things got to such a point as that is simply not collaboration or a healthy project at all. That is, I don't think a functional open source process exists under this clause being triggered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, missed this comment before.

I guess I'm also just trying to err on the side of encouraging collaboration over process

Always involving TSC to dismiss an objection wouldn't be encouraging collaboration over process though.

but agreed these rules are for when collaboration breaks down and having a clear non-process-based clause for moving forward does seem like it could help in such dire circumstances

We assume all collaborators are acting in good faith in our governance, if we remove that assumption we'd need to rewrite all of our governance documentation. Personally I don't see "the objector being unresponsive" in this case as a "dire circumstance", they might've just missed the notification or might be on a holiday*. "Unresponsive" is not the objector responding with "I still disagree with this change", but instead no interactions with the issue for seven days.

Again, collaboration over process, if an objector goes on holiday and indicates so on their GitHub profile or commenting on the PR, I'd expect the author to wait until the objector comes back before dismissing the objection.

That said, it seems like it could be healthier to remove any collaborator as a collaborator entirely if things got to such a point as that is simply not collaboration or a healthy project at all

In extreme cases where collaboration breaks down and becomes hostility between collaborators, the TSC and Moderation team should be notified and will step in to mediate the discussion. Removing a collaborator from the org is one of the most extreme actions we can take, and AFAIK we don't have any clear policies around it besides inactive collaborators (TSC is allowed to move inactive collaborators to Emeritus) and breach of privacy of Moderation reports. With that being said, the TSC is responsible for "maintaining the list of Collaborators", which could be interpreted as having the power/responsibility to remove someone if needed. Again, this is such an extreme case that if happened would probably damage the project healthiness for a while.

That is, I don't think a functional open source process exists under this clause being triggered

Not sure which clause you're referring to, can you elaborate?

With all that being said, I think we went too much in the weeds of an issue that rarely happens because of an isolated case that happened recently (which would've been prevented by the changes on this PR). Therefore I'll go ahead and land this, but feel free to reply if you want to continue the discussion.

Copy link
Contributor

Choose a reason for hiding this comment

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

100% of course it can be used in a collaborative way to dismiss an objection without needing full ceremony. That said there is a side of this which could be used in bad faith or as a last resort when communication breaks down but let's hope it never comes to that and if it does that things can be mediated properly through the TSC.


[Breaking changes](#breaking-changes) must receive
[TSC review](#involving-the-tsc). If two TSC members approve the pull request
Expand Down