[v4,08/12] Add guidelines about warnings

Submitted by Christophe de Dinechin on Feb. 15, 2018, 4:06 p.m.

Details

Message ID 20180215160625.75406-9-christophe@dinechin.org
State New
Headers show
Series "Updates to the style guide" ( rev: 3 ) in Spice

Not browsing as part of any series.

Commit Message

Christophe de Dinechin Feb. 15, 2018, 4:06 p.m.
From: Christophe de Dinechin <dinechin@redhat.com>

The objective of these guidelines is that:
- We avoid introducing new warnings
- We know how to fix old ones

Changes since v3:
- Put 'controversial' whitespace proposal in separate patch

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
 docs/spice_style.txt | 7 +++++++
 1 file changed, 7 insertions(+)

Patch hide | download patch | download mbox

diff --git a/docs/spice_style.txt b/docs/spice_style.txt
index 977ce154..90ad577d 100644
--- a/docs/spice_style.txt
+++ b/docs/spice_style.txt
@@ -523,3 +523,10 @@  using spice::streaming_agent::some_class;
 +
 [source,cpp]
 namespace ssa = spice::streaming_agent;
+
+Compilation
+-----------
+
+The source code should compile without warnings on all variants of GCC and clang available.
+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.

Comments

On Thu, Feb 15, 2018 at 05:06:21PM +0100, Christophe de Dinechin wrote:
> From: Christophe de Dinechin <dinechin@redhat.com>
> 
> The objective of these guidelines is that:
> - We avoid introducing new warnings
> - We know how to fix old ones
> 
> Changes since v3:
> - Put 'controversial' whitespace proposal in separate patch
> 
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
> ---
>  docs/spice_style.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/docs/spice_style.txt b/docs/spice_style.txt
> index 977ce154..90ad577d 100644
> --- a/docs/spice_style.txt
> +++ b/docs/spice_style.txt
> @@ -523,3 +523,10 @@ using spice::streaming_agent::some_class;
>  +
>  [source,cpp]
>  namespace ssa = spice::streaming_agent;
> +
> +Compilation
> +-----------
> +
> +The source code should compile without warnings on all variants of GCC and clang available.
> +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.

As said earlier, I don't think this brings much to explicitly states
this. And this is both too specific, while leaving some cases undefined,
this does not define a process to fix warnings which were not noticed at
the time the patch was ack'ed. But really, I would just drop this patch.
This could even be frightening to newcomers ("what, I have to test my
patch on dozens of compilers??"), while no regular committer is actually
going to do it.

Christophe