PR final steps (to squash or not to squash)

[René J.V. Bertin]

Mojca Miklavec wrote on 20161205::11:14:46 re: "Re: PR final steps (to squash or not to squash)"

This was meant to be a public reply, sending again.


>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.

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?

> 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.

> 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.

I know I'm very prone myself to address multiple aspect at once esp. when I'm taking over an abandoned port that's important for me (opencv, qca). You work on such a project, something that typically sees multiple commits in a personal repo, and without commit rights to the official tree you end up presenting a (more or less) finished version that can look very complicated if you only look at it as a patch w.r.t. the current verrsion.
IOW, it has more of a submission of a new port than a change to an existing one, and is much easier to evaluate as such.
Even if one can still describe the evolution in a chronological sequence of things one has changed it can be really untrivial to start over and recreate the corresponding patches (which one would have to test, which means introducing local regressions, etc).

R.