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

RFC: "Good to go on green" - allow an admin to say that this PR can be merged when it passes CI #10

Closed
orta opened this issue Aug 30, 2017 · 5 comments · Fixed by #59
Closed

Comments

@orta
Copy link
Contributor

orta commented Aug 30, 2017

Proposal:

Allow commenting in a PR that it's good to merge assuming CI passes.

Peril can listen to the status updates on a PR, then when they are all green - can check the PR comments to see if anyone say "Merge on green", if they have said it, then merge the PR.

Reasoning

My favourite GitLab feature is a tick box you have during review that this PR should be merged when it passes. I think the flow makes a lot of sense, CI can take a lot of time and remember to go back and merge after is annoying.

An alternative to this is that instead peril can send a slack DM to when it's green.

Exceptions:

It's hard to see this one being triggered by accident, but maybe Peril could confirm that it will merge on green to ensure that people aren't confused?

@jonallured
Copy link
Member

Would a special label work here? Like you said, it's hard to imagine this being triggered by accident, but if I think about actually doing this, I think I would probably review the PR, leave a comment thanking the person or whatever and then picking that special label seems like the right effort to invoke this feature.

@orta
Copy link
Contributor Author

orta commented Aug 30, 2017

That is a big improvement, thanks @jonallured, Peril can remove the label after too.

The interesting thing here is that labels are not org wide, but per-repo - in the past for CocoaPods @fabiopelosin built a script to automate setting up similar labels, maybe Peril can do the same.

@orta
Copy link
Contributor Author

orta commented Apr 27, 2018

I managed to get back to this in Danger's org, I agree with your setup: danger/peril-settings#6 (comment)

@orta
Copy link
Contributor Author

orta commented Apr 28, 2018

This is working on Danger's org. https://twitter.com/orta/status/990214980871098371

It requires using a newer version of Peril, so I might move Artsy to the staging server instead of our current (6 months old commit) heroku instance.

@jonallured
Copy link
Member

I'm excited to use this!! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants