[rudder-dev] Improving Pull Request merges and up-merging

[Jonathan Clarke]

Hi,

Let me start this discussion with a bit of context:

1) Currently, all changes to Rudder repos are done via GitHub Pull 
Requests. GitHub is a great tool for this kind of collaboration, and we 
see no reason to change from it, currently. It allows for easy 
discussion about the proposed change, directly in the code. Once the 
discussion has ended in a consensus, the proposed code change is merged 
directly from the GitHub web interface into the branch it's targeted for.

2) Currently, all Rudder repos have one branch per major version (2.11, 
3.0, 3.1, etc). All bug fixes are targeted to the oldest version in 
which the bug is present, then a semi-automatic process "up-merges" the 
fix from the older branches to the newer ones, ending up in master. This 
process is automatic when the merge is "fast-forward" (the change can be 
applied to the same "before" code in the upper branches), and is run 
automatically 3 times a day. However, when the up-merge generates a 
conflict (the "before" code has changed between branches), this requires 
a manual operation to up-merge the fix, outside of GitHub's web 
interface, directly using git CLI tools.

This situation causes some issues, currently:

 1. Often, several changes are merged into a repo before the manual
    operation to up-merge is done. This makes this manual operation even
    harder, because whoever does it needs to understand *all* the
    changes going in, and think about the consequences of all of them at
    the same time. This causes merges to sometimes not be done for days,
    until the right group of people is available. Sometimes, it can end
    up in a mistake (wrong line included/removed, merge markers left in,
    etc...)
 2. Often, the person reaction to automatic merge errors is a different
    one from whoever wrote or merged the PR. This means they don't know
    the code, and this will both make the manual up-merge harder and
    more error-prone.


Basically, the current approach is error-prone (there is a significant 
number of tickets opened to fix merge errors in Redmine), and causes 
unnecessary difficulty, thus delays. We've been looking for a way around 
this for a while, and in a meeting yesterday, we just came up with one.

The idea:

 1. Keep PR submission and review as they currently, but don't merge via
    GitHub's web interface.
 2. Once a consensus has been reached, instead of clicking on the
    "Merge" button in GitHub's web interface, the reviewer adds a tag to
    the PR that says something like "Ready for merge".
 3. At this stage, an automatic script will check out the PR, and merge
    it into the target branch, and test all up-merges to higher branches.
     1. If they are all "fast-forward" merges, the script will do the
        merges, and commit the result. Therefore, a change will
        simultaneously appear in all branches, the PR will be closed
        automatically (GitHub detects this). In this simple case, we're
        done, and the merge was almost immediate.
     2. If the up-merges require manual intervention, the script will
        comment on the PR. The PR author is then responsible for merging
        his own PR (alternatively, this can be the reviewer too, if the
        author does not have commit access). An interactive script will
        be developed (likely an extension of rudder-dev) that follows
        the same workflow as described in step 3.1 above, but letting
        the user interactively fix the conflicts. Then, the result will
        be commited simultaneously to all branches, the PR automatically
        closed.


The main benefit of this approach is clear: only authors and reviewers 
of the code will be doing the up-merges, thus reducing how error-prone 
these merges currently are once, and also only one PR will be up-merged 
at a time, thus reducing how error-prone these merges are again. Extra 
benefits include the option to customise the workflow after a PR is 
merged, including things like automatically updating the Redmine ticket, 
listing any contributors (commit authors) in the next changelog, etc...

There are some drawbacks, in particular that this will cause us to move 
away from the standard GitHub merging workflow. This is in fact one of 
the options that GitHub documents (if you read carefully next to the 
"Merge" button there is some text explaning you can do this on the 
command line). Also, this won't affect *contributors* workflow in 
GitHub, only committers, so targets a reduced population.

Benoit is working on a prototype, I believe.

What are your thoughts on this? Good idea, go, or bad idea, please don't 
do that? Any questions?

Jon
-- 
------------------------------------------------------------------------
*Logo Normation Jonathan CLARKE*
/Co-founder & Chief Product Officer/
Normation <http://www.normation.com>
------------------------------------------------------------------------
*87 rue de Turbigo, 75003 Paris, France*
Telephone: 	+33 (0)1 83 62 41 24
Mobile: 	+33 (0)6 99 60 03 10
------------------------------------------------------------------------

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.rudder-project.org/pipermail/rudder-dev/attachments/20160428/20f652e0/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/gif
Size: 1036 bytes
Desc: not available
URL: <http://www.rudder-project.org/pipermail/rudder-dev/attachments/20160428/20f652e0/attachment.gif>