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

Proposal: Not merging your own PRs without an explicit r+ or lgtm #1110

Closed
miketaylr opened this issue Jul 4, 2016 · 3 comments
Closed

Proposal: Not merging your own PRs without an explicit r+ or lgtm #1110

miketaylr opened this issue Jul 4, 2016 · 3 comments

Comments

@miketaylr
Copy link
Member

Right now the Submission Guidelines mention asking for review explicitly:

https://github.com/webcompat/webcompat.com/blob/master/CONTRIBUTING.md#submission-guidelines

I think we should add a section about not merging your own PR unless the person reviewing has left an explicit "r+" or "lgtm" comment. Possible ways this happens:

  1. Reviewee sends PR, asks for r?. Reviewer reviews, leaves r+.
  2. Reviewee sends PR, asks for r?. Reviewer reviews, points out issues, asks for changes. Reviewee makes changes, asks again for r?, repeat until r+.

(admins should be able to merge w/o r+ or lgtm, in the case of fixing broken builds, etc.)

We should be able to enforce this w/ a bot: https://github.com/blog/2051-protected-branches-and-required-status-checks, but for now we can just trust each other.

Thoughts? @webcompat/core-developers

@magsout
Copy link
Member

magsout commented Jul 4, 2016

👍💯
Adding the required lgtm or r+ combined with a bot should be a good thing. We avoid mistakes and the clicks on merge button by accident

@magsout
Copy link
Member

magsout commented Jul 4, 2016

I can take a look for adding a bot and update the documentation, If everyone agrees.

karlcow added a commit to karlcow/webcompat.com that referenced this issue Jul 5, 2016
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jul 5, 2016
@karlcow
Copy link
Member

karlcow commented Jul 5, 2016

@magsout for the bot, maybe that should be its own separate issue. I guess it will take a bit longer.
I provided a suggestion for the prose. Any edits/comments are welcome in the

@karlcow karlcow self-assigned this Jul 5, 2016
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jul 5, 2016
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jul 5, 2016
karlcow added a commit to karlcow/webcompat.com that referenced this issue Jul 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants