[Spice-devel] [PATCH v2 12/13] Add guidelines about warnings and whitespaces

[Lukáš Hrázký]

On Thu, 2018-02-08 at 11:09 +0100, Victor Toso wrote:
> Hi,
> 
> On Thu, Feb 08, 2018 at 05:01:05AM -0500, Frediano Ziglio wrote:
> > > > Depends on many cases. You don't want spurious changes to make harder to
> > > > look at the history for instance (that is a point for Nack).
> > > > The patch is not fixing anything or adding new feature (another for Nack).
> > > > On the other hand applied to code not changed for a long period (where is
> > > > unlikely to have to dig the history) or code with small history (where
> > > > is faster to skip in any case) changes.
> > > > Being style it depends also on personal opinions.
> > > 
> > > You can’t have it both ways. Here, you are simultaneously saying:
> > > 
> > > - We don’t want trailing whitespace
> > 
> > OT: Not only trailing, also tabs for instance.
> > 
> > > - We nack patches that fix trailing whitespace on their own
> > 
> > Not saying that, I'm saying is not black and white.
> 
> Because the code itself is inconsistent. It would be so much
> better to have a few patches that make the code consistent and
> then some git hook to check if given patch does not mess around
> the coding style instead of discussing this so many times over
> years.

Agreed!

My (partly philosofical) opinion on this is:

Code quality is more important than history (or diffs). If something is
wrong, just go ahead and fix it. If you want it fixed, it will be
recorded in the history. It's cleaner to have a separate commit for it
in the history. (that is obvious, right?)

Lukas