Re: Obstacles for using GitLab CI

[Jonas Hahnfeld]

Am Donnerstag, den 14.05.2020, 13:04 +0000 schrieb Carl Sorensen:
> 
> On 5/14/20, 2:31 AM, "lilypond-devel on behalf of Jonas Hahnfeld" <
> lilypond-devel-bounces+c_sorensen=address@hidden
>  on behalf of 
> address@hidden
> > wrote:
> 
> Am Donnerstag, den 14.05.2020, 08:09 +0100 schrieb James:
> > On a more general question, and not really understanding how this CI 
> > workflow will change 'contexturally' what we do, so apologies for if 
> > what I am about to say is ignorant, but are we still taking the 
> > 'master-must-always-be-good' / 
> > 'staging-can-be-bad-because-we-can-force-delete-not-just-revert' approach?
> > 
> > Rather than just automate everything and if something brakes we checkin 
> > a reversion which then makes the tree not 100% compilable?
> > 
> > I've liked that approach we take so far and it always instills a level 
> > of confidence about master.
> 
> We would keep 'master-must-always-be-good' but without having an
> explicit staging branch that we need to revert. Instead we ask GitLab
> to only add commits to master after they have passed testing. So
> staging + patchy is basically replaced by merge request + GitLab
> verifying the result of CI. My proposal will hopefully clear this up
> with concrete examples.
> 
> -> CS  I hope we can get back to the previous mode we worked in that not only 
> is master always good, but that every commit on master is known to work.
> 
> -> CS  As I have been verifying issues fixed for 2.20.1 I have found a number 
> of issues that were resolved with 2, 3, 4, or as many as 7 commits.  As far 
> as I know, we have no testing that indicates each of these commits builds 
> correctly, only that the full set of commits builds correctly.  We made a 
> conscious decision to require only single-commit fixes so that every commit 
> on master would build, and we could use git-bisect effectively to identify 
> regressions.
> 
> -> CS  I know that we had some discussion about how it is much more 
> convenient to have multiple small commits for reviewing, and I can see that 
> in nearly every case the multiple commits were aimed at helping reviewers 
> understand the patches, which I think is a great thing.  I'd be happy to keep 
> multiple commits for reviewing, but I believe we need to ensure that every 
> commit on master builds properly to support git-bisect.
> 
> -> CS  I hope that we can make our CI/merge process function such that either 
> we run the CI on every commit in a merge request or we rebase/squash/make a 
> single merge commit so that the intermediate, untested commits do not show up 
> on master.

I'm not aiming to do this, in my opinion it's a waste of resources to
run the full pipeline on every single commit. AFAIK GitLab CI doesn't
even support this?
I also oppose to saying that multiple smaller commits are bad and
everything should be squashed; there are enough guides out there
recommending the contrary and I think they are right: Commits are a
trace of logical changes. If a fix requires multiple changes that lend
themselves to splitting, this is favorable to almost any inspection of
code history.
(NB changes for review comments are not "logical" commits for me and
should be folded into the respective "base" commit.)

The ability to bisect is somewhat orthogonal: I agree that every commit
should build in principle, but eventually tooling updates or other
factors can still limit what you're able to compile in a years time.
Automated checks or even manual ones can't catch everything that could
go wrong in the future.

So in total this is a compromise between resources and the potential
benefit. I think testing at the level of merge requests (as we do right
now) is a good trade-off. It also ensure that master is always
compilable, at least in the way testing ensures.
All of this eventually comes down to trusting contributors to split the
changes in a meaningful way...

Jonas

signature.asc
Description: This is a digitally signed message part