Properly merging pull requests

Mojca Miklavec mojca at macports.org
Thu Nov 3 08:59:22 PDT 2016


Hi,

We have recently experienced problems with pull requests showing up as
rejected on the web interface rather than merged (while the changes
were in fact accepted, possibly with some modifications).

I admit my sins. In one case I did a minor modification (squashed
commits, removed the "Id" line, edited the commit message), while
basically everything else stayed the same as the author submitted and
everyone would have preferred if GitHub displayed "Merged" rather than
the red rejected sign.

We could have asked the author to keep fixing the branch forever, but
at some point it's simply faster and easier if an experienced
MacPorter just finishes the obvious rather than turning the users away
due to some stupid mistakes. The users can fix the mistakes in the
next PR.

In another case I (stupidly?) decided to add "Closes: #N" at the end
of the commit message (I thought it would be useful to have a link to
the original PR which is something that the committer cannot do anyway
without knowing the number of PR in advance). I probably also sinned
by thinking of "playing safe" and using cherry-pick rather than
"wrongly rebasing" and risking some undesired merge.

Can someone provide some guidelines of DOs and DONTs when accepting
(and modifying) pull requests? That should cover the cases that need:
- squashing commits
- minor edits of commit messages
- other minor edits of the sources

Thank you,
    Mojca


More information about the macports-dev mailing list