On 08/02/2010 10:41 AM, Kevin Wolf wrote:
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.
I won't argue that missing braces impact readability of the code, they
probably don't. However, as was pointed out in earlier discussion there
still remain two important points:
1. While it doesn't make a difference for the code itself, readability
of patches suffers when braces have to be added/removed when a second
line is inserted or disappears.
I understand the argument but it's not something that I strongly agree with.
2. I've messed up more than once with adding some debug code (even worse
when it happens with real code):
fprintf(stderr, "debug msg\n");
bar(); /* oops, not conditional any more */
This is hard to do with editors that auto indent unless you're copying and
pasting debugging. And yeah, I've made that mistake too doing the later :-)
This is why I tend to disagree with removing the rule, and suggest to
rather implement some automatic checks like Aurelien suggested (if we
need to change anything at all). I usually don't ask for a respin just
for braces if the patch is good otherwise, but if you think we should
just reject such patches without exceptions, I can change that.
Yeah, I try to remember to enforce it but often forget or just don't notice.
My eyes don't tend to catch missing braces like they would catch bad type
naming because the former is really not uncommon.
I'm happy with the status quo but I won't object to a git commit hook that