Patchwork Get coding style closer to the real world

login
register
mail settings
Submitter Alexander Graf
Date Nov. 30, 2009, 9:55 p.m.
Message ID <1259618155-4217-1-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/39860/
State New
Headers show

Comments

Alexander Graf - Nov. 30, 2009, 9:55 p.m.
Currently we have this stupid role of disallowing:

  if (r)
      break;

By disallowing this we clutter the code, making it less readable without
buying us anything. In fact, nobody actually sticks to this because it'd show
just how much bad taste the programmer doing this would have.

So IMHO we should change the coding style, so we can finally focus on
programming again and don't focus on useless coding style rules.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 CODING_STYLE |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)
Markus Armbruster - Dec. 1, 2009, 9:39 a.m.
Alexander Graf <agraf@suse.de> writes:

> Currently we have this stupid role of disallowing:
>
>   if (r)
>       break;
>
> By disallowing this we clutter the code, making it less readable without
> buying us anything. In fact, nobody actually sticks to this because it'd show
> just how much bad taste the programmer doing this would have.
>
> So IMHO we should change the coding style, so we can finally focus on
> programming again and don't focus on useless coding style rules.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>

ACK
Paul Brook - Dec. 4, 2009, 5:47 p.m.
On Monday 30 November 2009, Alexander Graf wrote:
> Currently we have this stupid role of disallowing:
> 
>   if (r)
>       break;

This has been discussed to death several times, in several different paces, 
and with no clear resolution or consensus, so I'm going to make an executive 
decision:

The coding style stays as-is. Braces are required. Please ensure that all 
patches follow the coding style. There may be exceptions, but there should be 
a *good* reason for deviation.


If there are real problems or ambiguities in the coding style then I am 
willing to consider fixing it. This particular change meets neither criteria.

By picking a single coding style we've pretty much guaranteed that most people 
will disagree with some of it. IMO consistency is more important.

Paul
Anthony Liguori - Dec. 4, 2009, 10:30 p.m.
Paul Brook wrote:
> On Monday 30 November 2009, Alexander Graf wrote:
>   
>> Currently we have this stupid role of disallowing:
>>
>>   if (r)
>>       break;
>>     
>
> This has been discussed to death several times, in several different paces, 
> and with no clear resolution or consensus, so I'm going to make an executive 
> decision:
>
> The coding style stays as-is. Braces are required. Please ensure that all 
> patches follow the coding style. There may be exceptions, but there should be 
> a *good* reason for deviation.
>
>
> If there are real problems or ambiguities in the coding style then I am 
> willing to consider fixing it. This particular change meets neither criteria.
>
> By picking a single coding style we've pretty much guaranteed that most people 
> will disagree with some of it. IMO consistency is more important.
>   

I agree 100%.

Regards,

Anthony Liguori

> Paul
>
>
>
Alexander Graf - Dec. 4, 2009, 11:47 p.m.
On 04.12.2009, at 18:47, Paul Brook wrote:

> On Monday 30 November 2009, Alexander Graf wrote:
>> Currently we have this stupid role of disallowing:
>> 
>>  if (r)
>>      break;
> 
> This has been discussed to death several times, in several different paces, 
> and with no clear resolution or consensus, so I'm going to make an executive 
> decision:
> 
> The coding style stays as-is. Braces are required. Please ensure that all 
> patches follow the coding style. There may be exceptions, but there should be 
> a *good* reason for deviation.
> 
> 
> If there are real problems or ambiguities in the coding style then I am 
> willing to consider fixing it. This particular change meets neither criteria.
> 
> By picking a single coding style we've pretty much guaranteed that most people 
> will disagree with some of it. IMO consistency is more important.

Could we please convert all the code base to the new style then? For exactly the consistency reason.

Alex
Avi Kivity - Dec. 5, 2009, 5:33 p.m.
On 12/05/2009 12:30 AM, Anthony Liguori wrote:
> Paul Brook wrote:
>> On Monday 30 November 2009, Alexander Graf wrote:
>>> Currently we have this stupid role of disallowing:
>>>
>>>   if (r)
>>>       break;
>>
>> This has been discussed to death several times, in several different 
>> paces, and with no clear resolution or consensus, so I'm going to 
>> make an executive decision:
>>
>> The coding style stays as-is. Braces are required. Please ensure that 
>> all patches follow the coding style. There may be exceptions, but 
>> there should be a *good* reason for deviation.
>>
>>
>> If there are real problems or ambiguities in the coding style then I 
>> am willing to consider fixing it. This particular change meets 
>> neither criteria.
>>
>> By picking a single coding style we've pretty much guaranteed that 
>> most people will disagree with some of it. IMO consistency is more 
>> important.
>
> I agree 100%.

+1'ed, and I note that although brace-happy code looks more cluttered it 
is a bit safer and produces cleaner patches.

Patch

diff --git a/CODING_STYLE b/CODING_STYLE
index a579cb1..806a1f8 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -51,11 +51,24 @@  QEMU coding style.
 
 4. Block structure
 
-Every indented statement is braced; even if the block contains just one
-statement.  The opening brace is on the line that contains the control
-flow statement that introduces the new block; the closing brace is on the
-same line as the else keyword, or on a line by itself if there is no else
-keyword.  Example:
+Usually indented statements are braced; even if the block contains just one
+statement. When braces reduce readability, they are not mandatory.
+
+Examples:
+
+    if (a == 5)
+        break;
+
+    if (b == 6) {
+        c = do_something();
+        /* Probably more code to be added here later */
+    }
+
+The opening brace is on the line that contains the control flow statement
+that introduces the new block; the closing brace is on the same line as
+the else keyword, or on a line by itself if there is no else keyword.
+
+Example:
 
     if (a == 5) {
         printf("a was 5.\n");