<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Janos,<br>
<br>
Thanks for sharing this feedback and the CI idea. We are in fact
already using that pattern, with automated tests built in to
several repos (see rudder-techniques/scripts/check-techniques.sh
and ncf/tests/acceptance/testall for example) that are run on each
change. In the future we intend to extend our CI system to run
these on appropriate Pull Requests for earlier test checking (and
to avoid committing something that doesn't pass tests).<br>
<br>
However, the use case about merging Pull Requests turned out to
not fit into this pattern well. It basically meant a lot of work
in Jenkins to "cheat" on which branches were used. In the end a
standalone Python script, that Benoît wrote about on this list
yesterday, is just about 100 lines, so simple and easy to
maintain.<br>
<br>
The future is more automated testing and automatic tests on Pull
Requests, for sure :)<br>
<br>
Jon<br>
<br>
<br>
On 04/29/2016 08:12 AM, Mattyasovszky János wrote:<br>
</div>
<blockquote
cite="mid:CAPz0yRvWRYirHX3Pyhni_bJqKi4Zx7E3Hf5j+tAKt6+TW8sjKw@mail.gmail.com"
type="cite">
<div dir="ltr">Hi All,
<div><br>
</div>
<div>First of all, thank you for investing work in making the
merge workflow more robust, I belive we all will benefit from
this.</div>
<div><br>
</div>
<div>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, <span style="line-height:1.5">like:</span></div>
<div><a moz-do-not-send="true"
href="https://github.com/jenkinsci/ghprb-plugin/blob/master/README.md">https://github.com/jenkinsci/ghprb-plugin/blob/master/README.md</a></div>
<div><span style="line-height:1.5"><a moz-do-not-send="true"
href="https://gitlab.com/gitlab-org/gitlab-ci-multi-runner">https://gitlab.com/gitlab-org/gitlab-ci-multi-runner</a></span><br>
</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>Regards,</div>
<div>Janos</div>
<div><br>
<div class="gmail_quote">
<div dir="ltr">Jonathan Clarke <<a moz-do-not-send="true"
href="mailto:jonathan.clarke@normation.com">jonathan.clarke@normation.com</a>>
ezt írta (időpont: 2016. ápr. 28., Cs, 16:28):<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"> Hi,<br>
<br>
Let me start this discussion with a bit of context:<br>
<br>
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.<br>
<br>
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.<br>
<br>
This situation causes some issues, currently:<br>
<br>
<ol>
<li>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...)<br>
</li>
<li>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.</li>
</ol>
<p><br>
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.<br>
</p>
<p>The idea:<br>
</p>
<ol>
<li>Keep PR submission and review as they currently,
but don't merge via GitHub's web interface.</li>
<li>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".</li>
<li>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.</li>
<ol>
<li>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.<br>
</li>
<li>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.</li>
</ol>
</ol>
<p><br>
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...<br>
</p>
<p>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.<br>
</p>
Benoit is working on a prototype, I believe.<br>
<br>
What are your thoughts on this? Good idea, go, or bad
idea, please don't do that? Any questions?<br>
<br>
Jon<br>
<div>-- <br>
<table width="380" border="0" cellpadding="0"
cellspacing="2">
<tbody>
<tr>
<td colspan="2">
<hr></td>
</tr>
<tr>
<td colspan="2"><b><img
src="cid:part4.05070806.08060208@normation.com"
alt="" class="" style="max-width: 100%;">
<span>Jonathan CLARKE</span></b><br>
<span><i>Co-founder & Chief Product
Officer</i></span><br>
<span><a moz-do-not-send="true"
href="http://www.normation.com"
target="_blank">Normation</a></span> </td>
</tr>
<tr>
<td colspan="2">
<hr></td>
</tr>
<tr>
<td colspan="2"><span><b>87 rue de Turbigo,
75003 Paris, France</b></span></td>
</tr>
<tr>
<td><span>Telephone:</span></td>
<td><span>+33 (0)1 83 62 41 24</span></td>
</tr>
<tr>
<td><span>Mobile:</span></td>
<td><span>+33 (0)6 99 60 03 10</span></td>
</tr>
<tr>
<td colspan="2">
<hr> </td>
</tr>
</tbody>
</table>
</div>
</div>
_______________________________________________<br>
rudder-dev mailing list<br>
<a moz-do-not-send="true"
href="mailto:rudder-dev@lists.rudder-project.org"
target="_blank">rudder-dev@lists.rudder-project.org</a><br>
<a moz-do-not-send="true"
href="http://www.rudder-project.org/mailman/listinfo/rudder-dev"
rel="noreferrer" target="_blank">http://www.rudder-project.org/mailman/listinfo/rudder-dev</a><br>
</blockquote>
</div>
</div>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
rudder-dev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:rudder-dev@lists.rudder-project.org">rudder-dev@lists.rudder-project.org</a>
<a class="moz-txt-link-freetext" href="http://www.rudder-project.org/mailman/listinfo/rudder-dev">http://www.rudder-project.org/mailman/listinfo/rudder-dev</a>
</pre>
</blockquote>
<br>
</body>
</html>