public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns
Date: Tue, 15 Aug 2017 12:57:34 +0200	[thread overview]
Message-ID: <972db03a-471e-9f6d-8bae-176ad06cc53d@redhat.com> (raw)
In-Reply-To: <CAKv+Gu-f3ijCjMaQbFiCU+OaQXD8vf_=N2b--Qt77c1jvhuAPg@mail.gmail.com>

On 08/12/17 00:52, Ard Biesheuvel wrote:
> On 11 August 2017 at 17:48, Laszlo Ersek <lersek@redhat.com> wrote:
>> We currently say "stick with 80 if it's convenient, extend to 120
>> otherwise".
> 
> It doesn't say that. It says you can make an exception for postfix
> comments, which is not unreasonable imo.

The way the section heading is worded (mentions 120 only), and how the
first paragraph starts ("preferably limit to 80"), are too easy to find
excuses out of.

> 
> This means most of the code in MdePkg/MdeModulePkg (afaik) already
> violates the old coding styile, so what good is it going to do to
> further restrict it?

It should affect new code.

(This is not the only example where we introduce a new style without
adapting the old code whole-sale. See for example EFI_D_* vs. DEBUG_*.)

Thanks
Laszlo

> 
>> This is too lax; much new edk2 code ignores the 80 columns
>> recommendation, resulting in source files that are hard to read for some
>> contributors. Remove the 120 columns excuse and make 80 columns a
>> requirement.
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  5_source_files/README.md | 17 ++++++++++++-----
>>  README.md                |  1 +
>>  2 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/5_source_files/README.md b/5_source_files/README.md
>> index a93492db4f0f..546d44d94fcb 100644
>> --- a/5_source_files/README.md
>> +++ b/5_source_files/README.md
>> @@ -33,12 +33,19 @@
>>
>>  ## 5.1 General Rules
>>
>> -### 5.1.1 Lines shall be 120 columns, or less
>> +### 5.1.1 Lines shall be 80 columns, or less
>>
>> -Preferably, limit line lengths to 80 columns or less. When this doesn't leave
>> -sufficient space for a good postfix style comment, extend the line to a total
>> -of 120 columns. Having some level of uniformity in the expected width of the
>> -source is useful for viewing and printing the code.
>> +Limit line lengths to 80 columns.
>> +
>> +Lines longer than 80 columns make it more difficult for the reader to find the
>> +beginning of the next line. They also tend to prevent users from displaying two
>> +source listings side-by-side on common display devices.
>> +
>> +When the 80 columns limit doesn't leave sufficient space for a postfix style
>> +comment, break the line into shorter segments at logical boundaries (for
>> +example, between the arguments of a function call, adhering to the spacing
>> +rules), or replace the postfix style comment with a standalone comment that
>> +precedes the statement.
>>
>>  ### 5.1.2 Do not use tab characters
>>
>> diff --git a/README.md b/README.md
>> index 8b9675b94937..8fad5a327b8c 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -112,3 +112,4 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved.
>>  | 2.1      | DRAFT for REFORMAT                                                                                                                                | 10/30/2015 |
>>  | 2.2      | Convert to Gitbook                                                                                                                                | June 2017  |
>>  |          | [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) [CCS] clarify line breaking and indentation requirements for multi-line function calls |            |
>> +|          | Limit lines to 80 columns                                                                                                                         |            |
>> --
>> 2.13.1.3.g8be5a757fa67
>>
>>



  parent reply	other threads:[~2017-08-15 10:55 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-11 16:48 [edk2-CCodingStandardsSpecification PATCH 0/2] improvements related to line wrapping Laszlo Ersek
2017-08-11 16:48 ` [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns Laszlo Ersek
2017-08-11 20:52   ` Jordan Justen
2017-08-11 21:01     ` Kinney, Michael D
2017-08-11 22:52   ` Ard Biesheuvel
2017-08-12  2:39     ` Jordan Justen
2017-08-12 10:03     ` Leif Lindholm
2017-08-15 10:57     ` Laszlo Ersek [this message]
2022-11-13  1:25       ` [edk2-devel] " Chang, Abner
2022-11-13  1:59         ` Michael D Kinney
2022-11-13  8:47           ` Chang, Abner
2017-08-11 16:48 ` [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments Laszlo Ersek
2017-08-11 20:45   ` Jordan Justen
2017-08-11 21:04     ` Kinney, Michael D
2017-08-12  1:31       ` Jordan Justen
2017-08-11 21:05     ` Andrew Fish
2017-08-12 10:13     ` Leif Lindholm
2017-08-15 11:16       ` Laszlo Ersek
2022-11-13  1:35         ` [edk2-devel] " Chang, Abner
2022-11-13  1:57           ` Michael D Kinney
2022-11-13  8:44             ` Chang, Abner
2022-11-13 17:36               ` Michael D Kinney
2022-11-14  1:09                 ` Chang, Abner
2022-11-14 17:07                   ` Michael D Kinney
2022-11-14 17:37                     ` Michael Kubacki
     [not found]                     ` <17278424C4A5D78F.32003@groups.io>
2022-11-14 18:05                       ` Michael Kubacki
2022-11-14 18:25                         ` Michael D Kinney
2022-11-14 18:49                           ` Michael Kubacki
2022-11-14 18:59                             ` Michael D Kinney
2022-11-14 19:08                             ` Sean
2022-11-15  2:38                               ` Chang, Abner
2017-08-11 17:07 ` [edk2-CCodingStandardsSpecification PATCH 0/2] improvements related to line wrapping Kinney, Michael D
2017-08-15 11:01   ` Laszlo Ersek
2017-08-15 15:17     ` Kinney, Michael D
2017-08-15 16:17       ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=972db03a-471e-9f6d-8bae-176ad06cc53d@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox