From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=54358 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OmksT-0000Kn-0q for qemu-devel@nongnu.org; Sat, 21 Aug 2010 06:00:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OmksR-0003ub-Fl for qemu-devel@nongnu.org; Sat, 21 Aug 2010 06:00:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44392) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OmksR-0003uO-82 for qemu-devel@nongnu.org; Sat, 21 Aug 2010 06:00:39 -0400 From: Markus Armbruster Subject: Re: [Qemu-devel] [PATCH 0/5] CODING_STYLE amendments References: <4C6A4291.1020105@redhat.com> <4C6A8CD8.2080701@codemonkey.ws> Date: Sat, 21 Aug 2010 11:54:11 +0200 In-Reply-To: (Blue Swirl's message of "Fri, 20 Aug 2010 20:24:53 +0000") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Jes Sorensen , Miguel Di Ciurcio Filho , qemu-devel Blue Swirl writes: > On Fri, Aug 20, 2010 at 6:44 PM, Blue Swirl wrote: >> On Fri, Aug 20, 2010 at 1:47 PM, Markus Armbruster wrote: >>> Anthony Liguori writes: >>>> To be perfectly honest, we have enough hard problems to solve in QEMU. >>>> We're spending a lot more time on coding style than we probably need >>>> to :-) >>> >>> In my not so humble opinion, that's because the current CODING_STYLE is >>> idiosyncratic, widely disliked (follows from idiosyncratic pretty much >>> inevitably), widely violated by existing code, and only haphazardly >>> enforced for new code. >> >> I think Coccinelle could help us here, it can check for some of the >> CODING_STYLE issues. We only need to include it to our build system >> and add git hooks if possible. It can also perform mechanical >> conversions (if desired). > > This Coccinelle script seems to do the job: [...] > There are some formatting issues though, I get changes like: > - for (i=0; i<5; i++) > + for(i = 0;i < 5;i++) { > > Reformatting the expressions with more spaces is nice, but removing > the spaces between 'for' and '(' and especially after ';' is not. Please make sure that patch submitters can easily check their patches with your tool. Depending on coccinelle isn't a problem for me, but it may well be for others. Unless you mass-convert existing code to your style, tools working on source files won't cut it, because reports of the patch's style violations are prone to drown in a sea of reports of preexisting style violations. There's a reason why Linux's scrtips/checkpatch.pl works on patch files. Even a working patch checking tool can only address the last issue (haphazard enforcement), not the other ones. You may not care. I still think inventing yet another idiosyncratic coding style plus tools to enforce it is a waste of time.