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

[Frediano Ziglio]

> 
> 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.

> 
> 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