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

Mattyasovszky János mail at matya.hu
Fri Apr 29 08:12:13 CEST 2016


Hi All,

First of all, thank you for investing work in making the merge workflow
more robust, I belive we all will benefit from this.

What you are describing however, looks very much like something that exist
in various forms, and is called CI system, which for example Jenkins or
gitlab's CI Runner are, not sure if it's worth to re-invent the wheel,
there are a lot of github plugins which are designed to interface with
gitlab and provide a nice way to interface with an automated build system,
like:
https://github.com/jenkinsci/ghprb-plugin/blob/master/README.md
https://gitlab.com/gitlab-org/gitlab-ci-multi-runner

This OTOH would require to write automated tests for the bugs / issues,
which can be tested for later re-occurrence. However, once the tests are
there, you could basically run automated tests on each commit / PR, so you
can ensure the code quality is on a stable level, and you don't have to do
so hard work on testing before releasing.

Well, yes, it requires compute resources to run through all tests, but
fortunately Jenkins is a widely used to have plenty of plugins which can be
used to add different methods of testing, so modelling any existing testing
workflow should be possible.

Regards,
Janos

Jonathan Clarke <jonathan.clarke at normation.com> ezt írta (időpont: 2016.
ápr. 28., Cs, 16:28):

> 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
> --
> ------------------------------
> * 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
> ------------------------------
> _______________________________________________
> rudder-dev mailing list
> rudder-dev at lists.rudder-project.org
> http://www.rudder-project.org/mailman/listinfo/rudder-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.rudder-project.org/pipermail/rudder-dev/attachments/20160429/aea6586c/attachment-0001.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/20160429/aea6586c/attachment-0001.gif>


More information about the rudder-dev mailing list