Re: [Qemu-devel] [PATCH] Drop braces around single statement rule

[Anthony Liguori]

On 07/31/2010 06:49 PM, Aurelien Jarno wrote:
> On Sat, Jul 31, 2010 at 08:23:55PM +0000, Blue Swirl wrote:
>    
> > On Sat, Jul 31, 2010 at 4:23 PM, malc<address@hidden>  wrote:
> >      
> > > History has shown that this particular rule is unenforcable.
> > >
> > > Signed-off-by: malc<address@hidden>
> > > ---
> > >   CODING_STYLE |   11 ++++++-----
> > >   1 files changed, 6 insertions(+), 5 deletions(-)
> > >        
> > Not again:
> > http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html
> >
> > There are plenty of ways to make the rule enforceable, for example we
> > could agree to start to revert commits which introduce new
> > CODING_STYLE violations.
> >
> >      
> It seems to be possible to add a pre-applypatch script to the git hook
> directory, that will verify the commit and reject it if it doesn't
> comply with the coding rules. Of course it's possible to commit a patch
> anyway by using --no-verify.
>    

There are certain aspects of CODING_STYLE that I think are pretty 
important.  For instance, space vs. tabs can really screw things up for 
people that have non-standard tabs.  This is something that enforcing at 
patch submission time seems to be really important.

Type naming seems important too because it's often not isolated.  IOW, a 
poor choice in one file can end up polluting other files quickly that 
require interacting.  The result is a mess of naming styles.

But something like braces around an if doesn't seem like it creates a 
big problem.  Most C programmers are used to seeing braces in some 
statements and not other.  Therefore, it's hard to argue that the code 
gets really unreadable if this isn't strictly enforced.

So really, I think the problem is that we're enforcing the words of 
CODING_STYLE instead of the intent.  The intent of CODING_STYLE is to 
improve the readability of the code.  I think it requires a certain 
amount of taste to be applied.

Rejecting a big patch because braces aren't used in single line if 
statements seems to be an unnecessary barrier to me.

Regards,

Anthony Liguori

> The good point of this approach is that the rule is enforced by a
> script, which is not suppose to make mistakes, and that it can be shared
> between patch submitters and patch committers: both side can make
> mistakes and it is always better to know that as early as possible.
>
> Of course someone as to translate the coding rules in a set of regular
> expressions able to catch errors.
>
>