-
Notifications
You must be signed in to change notification settings - Fork 166
Policies
-
Branches behind Pull Request (PR) should be rebased to latest 'master' before merging the PR. Reason is that this avoids a hard to read history with lots of parallel branches.
-
Pull Request should be merged by pressing the "Merge" button in the github UI. Reasons are because that way the branch is "grouped" by the resulting merge commit and, more important, the merging commit message has some pointer to the related PR and the discussion that might have happened on the PR.
-
Pull Request should be merged by the person which raised the PR. Reasons are because that enables that person to squash any fixup commits created during review, also rebase to latest 'master', if needed, and allows to do another possible update to the PR, if wanted. Most important: getting paid for his patch by satisfaction from pressing the "big green button", right.
-
Pull Requests can be merged if one trusted (define yourself) person commented on the PR with an explicit "Ship it" or "Merge it".
Commits that are considered to not need review can be committed directly to 'master', without doing some PR dance. Again they should be done against the latest 'master', so the history is a single thread.
For PRs where review has started already, at least for larger ones any fixes and additions should be done by separate commits, so that reviewers can quickly see which updates have been made. This avoid having to re-read and understand the whole PR again if just some minor thing was changed.
Such fixup commits would be done with explicit markers in the commit message that they are fixup commits and which other commit they will be squashed into. Once the review has been finished, the fixup commits would be squashed before the final merge is done.
Usually the person who opened an issue should close it. (github automatically closing issues screws that slightly)