Re: [PATCH v2 12/13] Add guidelines about warnings and whitespaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]