From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=43279 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ofwp8-0004BM-4O for qemu-devel@nongnu.org; Mon, 02 Aug 2010 11:21:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Ofwp3-0007Si-P3 for qemu-devel@nongnu.org; Mon, 02 Aug 2010 11:21:06 -0400 Received: from mail-qw0-f45.google.com ([209.85.216.45]:38338) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Ofwp3-0007SP-Ml for qemu-devel@nongnu.org; Mon, 02 Aug 2010 11:21:01 -0400 Received: by qwi4 with SMTP id 4so1723975qwi.4 for ; Mon, 02 Aug 2010 08:21:00 -0700 (PDT) Message-ID: <4C56E253.7010403@codemonkey.ws> Date: Mon, 02 Aug 2010 10:20:51 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] Drop braces around single statement rule References: <1280593414-2232-1-git-send-email-av1474@comtv.ru> <20100731234956.GJ31640@hall.aurel32.net> In-Reply-To: <20100731234956.GJ31640@hall.aurel32.net> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: Blue Swirl , qemu-devel@nongnu.org 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 wrote: >> >>> History has shown that this particular rule is unenforcable. >>> >>> Signed-off-by: malc >>> --- >>> 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. > >