>
> If we do it, we'll end up in weird situations which will be annoying for
> everyone
Such as? I'm not disputing, but if we're to assess the relative
strengths/weaknesses, we need to have specifics to discuss.
If we do go with this suggestion, we will most likely want to enable a
shared git rerere cache, so that rebasing is not painful when there are
future commits.
If instead we go with "repairing" commits, we cannot have a "queue" of
things to merge up to. Say you have a string of commits waiting for
approval C1 to C4; you made C1, and it broke something. You introduce C5 to
fix it, but the tests are still broken. Did you not really fix it? Or
perhaps one of C2 to C4 are to blame, but which? And have you accidentally
broken *them* with your commit? Who knows. Either way, we definitely cannot
fast forward. At the very best we can hope that the new merge did not
conflict or mess up the other people's C2 to C4 commits, and they have to
now merge on top. But what if another merge comes in, C6, in the meantime;
and C2 really did also break the tests in some way; how do we determine C2
was to blame, and not C6, or C3 or C4? What do the committers for each of
these do? We end up in a lengthy tussle, and aren't able to commit any of
these to the mainline until all of them are resolved. Really we have to
prevent any merges to the staging repository until the mistakes are fixed.
Since our races in these scenario are the length of time taken for cassci
to vet them, these problems are much more likely than current race to
commit.
In the scheme I propose, in this scenario, the person who broke the build
rebases everyone's branches to his now fixed commit, and the next broken
commit gets blamed, and all other commits being merged in on top can go in
smoothly. The only pain point I can think of is the multi-branch rebase,
but this is solved by git rerere.
I agree running tests is painful, but at least for the build, this should
> be the responsibility of the committer to build before merging
Why make the distinction if we're going to have staging commits? It's a bit
of a waste of time to run three ant real-clean && ant tasks, and increases
the race window for merging (which is painful whether or not involves a
rebase), and it is not a *typical* occurrence ("alarming" is subjective)
On Thu, May 7, 2015 at 2:12 PM, Sylvain Lebresne <sylv...@datastax.com>
wrote:
> > If one of them breaks, we go and edit the _staging branch in place to
> correct
> > the problem, and let CI run again.
>
> I would strongly advise against *in place* edits. If we do it, we'll end up
> in
> weird situations which will be annoying for everyone. Editing commits that
> have
> been shared is almost always a bad idea and that's especially true for
> branch
> that will have some amount of concurrency like those staging branches.
>
> Even if such problems are rare, better to avoid them in the first place by
> simply
> commit new "fixup" commits as we currently do. Granted this give you a
> slightly
> less clean history but to the best of my knowledge, this hasn't been a pain
> point so far.
>
> > wait for CI; all clear
> >
> > git checkout cassandra-2.0; git merge cassandra-2.0_staging
> > git checkout cassandra-2.1; git merge cassandra-2.1_staging
> > git checkout trunk; git merge trunk_staging
> >
> > This does introduce some extra steps to the merge process
>
> If we do this, we should really automate that last part (have the CI
> environment merge the staging branch to the non-staging ones on success).
>
> > It seems if we want an "always releasable" set of branches, we need
> something
> > along these lines.
>
> Agreed as far as having staging branches vetoed by CI goes. Less sure about
> the edit-commit-in-place part as said above.
>
> > I certainly break tests by mistake, or the build itself, with alarming
> regularity.
>
> I agree running tests is painful, but at least for the build, this should
> be
> the responsibility of the committer to build before merging. We all forget
> it from
> times to times and that's ok, but it's not ok if it's "alarmingly regular".
>
> --
> Sylvain
>
