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

Merging PRs #36

Closed
jimmynotjim opened this issue Apr 14, 2015 · 5 comments
Closed

Merging PRs #36

jimmynotjim opened this issue Apr 14, 2015 · 5 comments
Labels

Comments

@jimmynotjim
Copy link
Contributor

A question as to who should be merging PRs came up in #35. The dev manual says:

Don’t: Merge your own pull requests!

But within Flapjack we've found there are a few good reasons to merge your own requests after the appropriate number of sign-offs (we chose two reviewers)

  • gives author a chance to cleanup and squash any edits (if desired);
  • fix conflicts (sometimes there's a conflict, you fix it, then no one get's around to merging and new conflicts come up, creating a cycle of "please fix conflict" comments and related commits);
  • merge when appropriate (sometimes multiple PRs need to go together).

In addition to this, merging our own PRs after being properly reviewed follows the bit about being trusted as a technology professional discussed in the Pain Points issue - https://[GHE]/sheltonw/2014-performance-kanban/issues/63#issuecomment-38650

@ascott1
Copy link
Member

ascott1 commented Apr 17, 2015

I think this is definitely a conversation worth having, but one we should have with the team at large (not just the front-end team).

@mistergone
Copy link
Member

Time travel! Almost four months later, I'll add that our Team decided to merge our own pull requests after requiring reviews. This puts the responsibility of getting a PR through on the person who made it, instead of a PR floating in space until someone decides to look at it. It means the Pull Requester has to get Teammates to review and comment so their code gets included.

@jimmynotjim
Copy link
Contributor Author

Trend setters!! I think this is something we should really be bringing back up, and @ascott1 you're right it should be an org level discussion. Maybe we can schedule a conversation at the next Dev Cop after @virtix is back from leave? Or should we just start an internal issue in D&D?

@ascott1
Copy link
Member

ascott1 commented Aug 25, 2015

Maybe we can schedule a conversation at the next Dev Cop after @virtix is back from leave? Or should we just start an internal issue in D&D?

I'd vote yes to both of these. Maybe start with the issue and set a hard deadline of "we're going to talk about and finalize this at the Dev CoP meeting on xx/xx/xx."

@jimmynotjim
Copy link
Contributor Author

Opened an internal discussion in the handbook repo, issue 167 (sorry, no links)

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

No branches or pull requests

4 participants