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

[Lukáš Hrázký]

On Thu, 2018-02-08 at 04:02 -0500, Frediano Ziglio wrote:
> > 
> > From: Christophe de Dinechin <dinechin at redhat.com>
> > 
> > The objective of these guidelines is that:
> > - We avoid introducing new warnings
> > - We know how to fix old ones
> > - We don't have to isolate whitespace changes when submitting patches,
> >   i.e. someone who use tools that automatically strip whitespaces and
> >   therefore "repairs" earlier errors should not be punished for it.
> 
> Sorry, I don't agree with the automatic tool, patches should not
> contain extra changes unless they fix space changes while changing
> these lines for other reasons.
> I'll personally accept single patches fixing the spaces.

I'm with Frediano here. If you want to automatically fix whitespace
errors, you can do it in a separate commit without much effort?

I can also go right now and fix all trailing whitespaces with a bash
oneliner, submit the patch and we have a moot point here? :)

Lukas

> > 
> > Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
> > ---
> >  docs/spice_style.txt | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/docs/spice_style.txt b/docs/spice_style.txt
> > index e2465aa9..d9644f9a 100644
> > --- a/docs/spice_style.txt
> > +++ b/docs/spice_style.txt
> > @@ -404,3 +404,12 @@ Also in source (no header) files you must include
> > `config.h` at the beginning so
> >  
> >  #include "spice_server.h"
> >  ----
> > +
> > +
> > +Compilation
> > +-----------
> > +
> > +The source code should compile without warnings on all variants of GCC and
> > clang available.
> 
> This is quite strong, looks like before sending a patch you should
> use any variant you find.
> 
> I would go with a more open specification not including the compilers:
> 
> "The source code should compile without warnings"
> 
> > +A patch may be rejected if it introduces new warnings.
> > +Warnings that appear over time due to improvements in compilers should be
> > fixed in dedicated patches. A patch should not mix warning fixes and other
> > changes.
> 
> agreed
> 
> > +Any patch may adjust whitespace (e.g. eliminate trailing whitespace).
> > Whitespace adjustments do not require specific patches.
> 
> don't agree
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel