|
From: | Anthony Liguori |
Subject: | Re: [Qemu-devel] [PATCH] Drop braces around single statement rule |
Date: | Mon, 02 Aug 2010 10:20:51 -0500 |
User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.11) Gecko/20100713 Lightning/1.0b1 Thunderbird/3.0.6 |
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.
[Prev in Thread] | Current Thread | [Next in Thread] |