[flashrom] Future flashrom development

[David Hendricks]

On Wed, Oct 25, 2017 at 4:04 AM, Nico Huber <nico.h at gmx.de> wrote:
>
> On 23.10.2017 09:39, Stefan Tauner wrote:
>>
>> On Fri, 13 Oct 2017 17:42:11 +0200
>> Nico Huber <nico.h at gmx.de> wrote:
>>
>>> What I didn't realize last night: the `staging` branch contains
>>> valuable information in lots of fixup! commits that would be lost
>>> if we don't keep `staging`. They look ugly in the log but their
>>> messages still contain some reasoning about the changes. I prefer
>>> to use the current `staging` branch as `master` therefore.
>>
>>
>> *If* there is valuable information in them then they should get into
>> the final commit message (i.e. the one that squashes everything to
>> stable). I deem them basically as part of the review process of the
>> whole change (i.e. the commit eventually committed to stable) and sure,
>> there is information in it that might be interesting but this likewise
>> applies to all review comments and I would not want them to be part of
>> the commit messages, would you?
>
>
> No, ofc not. But there is always a compromise of what to include into
> the commit message. Though, maybe I'd write too elaborate ones, I think
> yours are often too thin (it only doesn't look like it because you do
> too much in single commits, IMHO).

This is another area where Gerrit helps us. Each patch gets a
"Reviewed-on:" line appended which makes it easy refer to review
comments when additional context is needed. If we squash several
patches into one we lose that context. We could have multiple
"Reviewed-on:" lines in squashed patches, but that would be sort of
awkward.

>> For someone not
>>     involved in the respective review/development process (i.e.
>>     everybody but the author of the fixups and respective reviewer(s)) it
>>     is very hard to track what the actual outcome of all of the fixups
>>     is.
>
>
> In the git way of fixups (e.g. `fixup! <original commit>`), this is
> pretty easy: The outcome is what the original commit message promised.

Also, fixups and refactoring should not be conflated. Fixups should
only ensure the intended outcome of the original patch is met.
Refactoring code, for example simplification, cosmetic changes, etc.
should be done in separate patches anyway.

>>     Both can make bisecting for specific problems tedious if unlucky.
>
>
> Lol, that's still better than squashing commits into bigger ones which
> makes bisecting less feasible.

Squashing into uber-patches also defeats the purpose of `git blame`.

Squashing approach has a lot of significant drawbacks - it breaks `git
bisect`, it breaks `git blame`, and depending on who is doing the
squashing we might lose references to code reviews from "Reviewed-on:"
lines. It also requires a lot of time and manual effort which leads to
arbitrary decisions and possibly more errors. I just don't see why we
would go thru all that when git and gerrit make things so simple and
without all the manual steps.