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

Christophe Fergeau cfergeau at redhat.com
Wed Feb 14 16:43:12 UTC 2018


On Thu, Feb 08, 2018 at 11:09:35AM +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.

Yup, would be good to have...
Regarding whitespace changes, I don't think we have many trailing
whitespace problems in spice*, probably a bit more tabs here and there.
Regarding fixing whitespace, I'll generally ack patches fixing one or 2
unrelated whitespace issues in a function the commit is modifying, or
a patch fixing the whitespace issues in a file the patch series is going
to change. Not sure what I would say to a patch fixing this over all the
codebase (mostly depends on how big it would be :)

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180214/6d64ad99/attachment.sig>


More information about the Spice-devel mailing list