PR final steps (to squash or not to squash)

[Rainer Müller]

On 2016-12-05 13:37, René J.V. Bertin wrote:
> Mojca Miklavec wrote on 20161205::11:14:46 re: "Re: PR final steps
> (to squash or not to squash)"

>> We discussed this quite a bit (I'm not sure where) and the
>> conclusion was that: - we want a linear history (therefore we do
>> rebasing) - we want to avoid "weird commits" (so yes, we want to
>> squash the commits)
> 
> I guess time and practice will tell, but that does sound like having
> a different patch review mechanism might be a good idea at some point
> in the future. Preferably something that makes it a bit easier to
> test a patch before it is committed.

What would be easier than just checking out the updated Portfile? You
can also just download the patch and apply it. Open for suggestions.

We migrated to GitHub because people were specifically asking for the
ability to open pull requests instead of attaching patches in Trac.

> Did the discussion include consideration of the fact that each commit
> can send the build bots on a rebuilding spree (AFAIK it does, at
> least), something one would probably prefer to avoid?

Just needs someone to work on it, the discussion on this took place on
this mailing list.

However, our mailing list archives are still broken after the move off
of macOS forge, so I cannot link to this directly.

From: Eric A. Borisch
Subject: Another workflow (pull requests) question.
Date: Mon, 14 Nov 2016 10:51:21 -0600
Message-Id:
<CAASnNnrtmz5-8cMSsLmNen-MN9O+UzEqjq=f0DeHMC=GPVMqXw at mail.gmail.com>

>> Usually "larryv" is the one who takes most care to split commits
>> into "reasonable atomic commits".
> 
> All due respect and all that, but this is error-prone, opens the door
> to "re-interpretation" of the intended changes ... and I guess we all
> have better things to do than this kind of task.

Well, it gets easier if the pull request author takes care of this.

>> A typical example would be when you fix whitespaces in the first
>> commit, fix typos in the second commit and add a variant in the
>> third commit.
> 
> A typical example indeed, but how justified is it to be this
> "pedantic" (for lack of a better term!) here? I can see some
> justification for it in (very) complex software where you may be
> patching multiple files at once, but Portfiles are by definition
> single-file programs that are rarely of true complexity (rather, much
> complexity is hidden in "base"). OTOH it *is* true that whitespace
> changes can have more side-effects in Tcl than they have in C, but
> that's also a possible source of error when a 3rd party starts
> splitting up more elaborate change sets.

Is this supposed to be an argument for squashing or against?

In my opinion, it does not make sense to keep every little change that
was raised in reviews in a separate commit. That would be a lot of noise
in the history and is usually not relevant.

In any case, if you want to retain history, every commit has to follow
our commit message guidelines.

Rainer