diff mbox series

[RFC] devtools: increase default line length to 100

Message ID 20200608164640.189755-1-bruce.richardson@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series [RFC] devtools: increase default line length to 100 |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch warning coding style issues

Commit Message

Bruce Richardson June 8, 2020, 4:46 p.m. UTC 
  Rather than continuing to recommend an 80-char line limit, let's take a hint
from the Linux kernel[1] and aim for an 100-char recommended limit instead.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 devtools/checkpatches.sh                 | 2 +-
 doc/guides/contributing/coding_style.rst | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Stephen Hemminger June 8, 2020, 7:17 p.m. UTC | #1 
On Mon,  8 Jun 2020 17:46:40 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> Rather than continuing to recommend an 80-char line limit, let's take a hint
> from the Linux kernel[1] and aim for an 100-char recommended limit instead.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  devtools/checkpatches.sh                 | 2 +-
>  doc/guides/contributing/coding_style.rst | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index 158087f1c..4970ed830 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -15,7 +15,7 @@ VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
>  # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL to a valid path
>  # to a dictionary.txt file if dictionary.txt is not in the default location.
>  codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
> -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
> +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
>  
>  # override default Linux options
>  options="--no-tree"
> diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
> index 4efde93f6..1db3a7bbe 100644
> --- a/doc/guides/contributing/coding_style.rst
> +++ b/doc/guides/contributing/coding_style.rst
> @@ -21,7 +21,7 @@ The rules and guidelines given in this document cannot cover every situation, so
>  * In the case of creating new files, the style should be consistent within each file in a given directory or module.
>  * The primary reason for coding standards is to increase code readability and comprehensibility, therefore always use whatever option will make the code easiest to read.
>  
> -Line length is recommended to be not more than 80 characters, including comments.
> +Line length is recommended to be not more than 100 characters, including comments.
>  [Tab stop size should be assumed to be 8-characters wide].
>  
>  .. note::

I would even support going to 120 characters.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Jerin Jacob June 9, 2020, 4:42 a.m. UTC | #2 
On Tue, Jun 9, 2020 at 12:47 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon,  8 Jun 2020 17:46:40 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> > Rather than continuing to recommend an 80-char line limit, let's take a hint
> > from the Linux kernel[1] and aim for an 100-char recommended limit instead.
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  devtools/checkpatches.sh                 | 2 +-
> >  doc/guides/contributing/coding_style.rst | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> > index 158087f1c..4970ed830 100755
> > --- a/devtools/checkpatches.sh
> > +++ b/devtools/checkpatches.sh
> > @@ -15,7 +15,7 @@ VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
> >  # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL to a valid path
> >  # to a dictionary.txt file if dictionary.txt is not in the default location.
> >  codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
> > -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
> > +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
> >
> >  # override default Linux options
> >  options="--no-tree"
> > diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
> > index 4efde93f6..1db3a7bbe 100644
> > --- a/doc/guides/contributing/coding_style.rst
> > +++ b/doc/guides/contributing/coding_style.rst
> > @@ -21,7 +21,7 @@ The rules and guidelines given in this document cannot cover every situation, so
> >  * In the case of creating new files, the style should be consistent within each file in a given directory or module.
> >  * The primary reason for coding standards is to increase code readability and comprehensibility, therefore always use whatever option will make the code easiest to read.
> >
> > -Line length is recommended to be not more than 80 characters, including comments.
> > +Line length is recommended to be not more than 100 characters, including comments.
> >  [Tab stop size should be assumed to be 8-characters wide].
> >
> >  .. note::
>
> I would even support going to 120 characters.
>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Jerin Jacob <jerinj@marvell.com>
Bruce Richardson June 9, 2020, 9:29 a.m. UTC | #3 
On Mon, Jun 08, 2020 at 12:17:23PM -0700, Stephen Hemminger wrote:
> On Mon,  8 Jun 2020 17:46:40 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > Rather than continuing to recommend an 80-char line limit, let's take a hint
> > from the Linux kernel[1] and aim for an 100-char recommended limit instead.
> > 
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  devtools/checkpatches.sh                 | 2 +-
> >  doc/guides/contributing/coding_style.rst | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> > index 158087f1c..4970ed830 100755
> > --- a/devtools/checkpatches.sh
> > +++ b/devtools/checkpatches.sh
> > @@ -15,7 +15,7 @@ VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
> >  # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL to a valid path
> >  # to a dictionary.txt file if dictionary.txt is not in the default location.
> >  codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
> > -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
> > +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
> >  
> >  # override default Linux options
> >  options="--no-tree"
> > diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
> > index 4efde93f6..1db3a7bbe 100644
> > --- a/doc/guides/contributing/coding_style.rst
> > +++ b/doc/guides/contributing/coding_style.rst
> > @@ -21,7 +21,7 @@ The rules and guidelines given in this document cannot cover every situation, so
> >  * In the case of creating new files, the style should be consistent within each file in a given directory or module.
> >  * The primary reason for coding standards is to increase code readability and comprehensibility, therefore always use whatever option will make the code easiest to read.
> >  
> > -Line length is recommended to be not more than 80 characters, including comments.
> > +Line length is recommended to be not more than 100 characters, including comments.
> >  [Tab stop size should be assumed to be 8-characters wide].
> >  
> >  .. note::
> 
> I would even support going to 120 characters.
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

I think 100 is enough.

In my case, I have a 1080p 24" monitor, and with two terminals side-by-side
100 characters just fits inside each vim window. Going to 120 would be fine
for single terminal at a time, but I would find awkward for e.g.
side-by-side diff comparison in meld etc.
Ananyev, Konstantin June 9, 2020, 10 a.m. UTC | #4 
> 
> On Mon, Jun 08, 2020 at 12:17:23PM -0700, Stephen Hemminger wrote:
> > On Mon,  8 Jun 2020 17:46:40 +0100
> > Bruce Richardson <bruce.richardson@intel.com> wrote:
> >
> > > Rather than continuing to recommend an 80-char line limit, let's take a hint
> > > from the Linux kernel[1] and aim for an 100-char recommended limit instead.
> > >
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >  devtools/checkpatches.sh                 | 2 +-
> > >  doc/guides/contributing/coding_style.rst | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> > > index 158087f1c..4970ed830 100755
> > > --- a/devtools/checkpatches.sh
> > > +++ b/devtools/checkpatches.sh
> > > @@ -15,7 +15,7 @@ VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
> > >  # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL to a valid path
> > >  # to a dictionary.txt file if dictionary.txt is not in the default location.
> > >  codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
> > > -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
> > > +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
> > >
> > >  # override default Linux options
> > >  options="--no-tree"
> > > diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
> > > index 4efde93f6..1db3a7bbe 100644
> > > --- a/doc/guides/contributing/coding_style.rst
> > > +++ b/doc/guides/contributing/coding_style.rst
> > > @@ -21,7 +21,7 @@ The rules and guidelines given in this document cannot cover every situation, so
> > >  * In the case of creating new files, the style should be consistent within each file in a given directory or module.
> > >  * The primary reason for coding standards is to increase code readability and comprehensibility, therefore always use whatever option
> will make the code easiest to read.
> > >
> > > -Line length is recommended to be not more than 80 characters, including comments.
> > > +Line length is recommended to be not more than 100 characters, including comments.
> > >  [Tab stop size should be assumed to be 8-characters wide].
> > >
> > >  .. note::
> >
> > I would even support going to 120 characters.
> >
> > Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> I think 100 is enough.
> 
> In my case, I have a 1080p 24" monitor, and with two terminals side-by-side
> 100 characters just fits inside each vim window. Going to 120 would be fine
> for single terminal at a time, but I would find awkward for e.g.
> side-by-side diff comparison in meld etc.

My preference would be to keep things as it is - 80 chars per line.
Having multiple different formatting styles in one source file
looks really awkward and make it hard to follow.
Andrew Rybchenko June 9, 2020, 1:40 p.m. UTC | #5 
On 6/9/20 1:00 PM, Ananyev, Konstantin wrote:
> 
>>
>> On Mon, Jun 08, 2020 at 12:17:23PM -0700, Stephen Hemminger wrote:
>>> On Mon,  8 Jun 2020 17:46:40 +0100
>>> Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>
>>>> Rather than continuing to recommend an 80-char line limit, let's take a hint
>>>> from the Linux kernel[1] and aim for an 100-char recommended limit instead.
>>>>
>>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
>>>>
>>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>>>> ---
>>>>  devtools/checkpatches.sh                 | 2 +-
>>>>  doc/guides/contributing/coding_style.rst | 2 +-
>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
>>>> index 158087f1c..4970ed830 100755
>>>> --- a/devtools/checkpatches.sh
>>>> +++ b/devtools/checkpatches.sh
>>>> @@ -15,7 +15,7 @@ VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
>>>>  # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL to a valid path
>>>>  # to a dictionary.txt file if dictionary.txt is not in the default location.
>>>>  codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
>>>> -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
>>>> +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
>>>>
>>>>  # override default Linux options
>>>>  options="--no-tree"
>>>> diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
>>>> index 4efde93f6..1db3a7bbe 100644
>>>> --- a/doc/guides/contributing/coding_style.rst
>>>> +++ b/doc/guides/contributing/coding_style.rst
>>>> @@ -21,7 +21,7 @@ The rules and guidelines given in this document cannot cover every situation, so
>>>>  * In the case of creating new files, the style should be consistent within each file in a given directory or module.
>>>>  * The primary reason for coding standards is to increase code readability and comprehensibility, therefore always use whatever option
>> will make the code easiest to read.
>>>>
>>>> -Line length is recommended to be not more than 80 characters, including comments.
>>>> +Line length is recommended to be not more than 100 characters, including comments.
>>>>  [Tab stop size should be assumed to be 8-characters wide].
>>>>
>>>>  .. note::
>>>
>>> I would even support going to 120 characters.
>>>
>>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>>
>> I think 100 is enough.
>>
>> In my case, I have a 1080p 24" monitor, and with two terminals side-by-side
>> 100 characters just fits inside each vim window. Going to 120 would be fine
>> for single terminal at a time, but I would find awkward for e.g.
>> side-by-side diff comparison in meld etc.
> 
> My preference would be to keep things as it is - 80 chars per line.
> Having multiple different formatting styles in one source file
> looks really awkward and make it hard to follow.

+1
Bruce Richardson June 9, 2020, 1:57 p.m. UTC | #6 
On Tue, Jun 09, 2020 at 04:40:28PM +0300, Andrew Rybchenko wrote:
> On 6/9/20 1:00 PM, Ananyev, Konstantin wrote:
> > 
> >>
> >> On Mon, Jun 08, 2020 at 12:17:23PM -0700, Stephen Hemminger wrote:
> >>> On Mon,  8 Jun 2020 17:46:40 +0100 Bruce Richardson
> >>> <bruce.richardson@intel.com> wrote:
> >>>
> >>>> Rather than continuing to recommend an 80-char line limit, let's
> >>>> take a hint from the Linux kernel[1] and aim for an 100-char
> >>>> recommended limit instead.
> >>>>
> >>>> [1]
> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> >>>>
> >>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
> >>>> devtools/checkpatches.sh                 | 2 +-
> >>>> doc/guides/contributing/coding_style.rst | 2 +- 2 files changed, 2
> >>>> insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> >>>> index 158087f1c..4970ed830 100755 --- a/devtools/checkpatches.sh +++
> >>>> b/devtools/checkpatches.sh @@ -15,7 +15,7 @@
> >>>> VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
> >>>> # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL
> >>>> to a valid path # to a dictionary.txt file if dictionary.txt is not
> >>>> in the default location.
> >>>> codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
> >>>> -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
> >>>> +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
> >>>>
> >>>>  # override default Linux options options="--no-tree" diff --git
> >>>>  a/doc/guides/contributing/coding_style.rst
> >>>>  b/doc/guides/contributing/coding_style.rst index
> >>>>  4efde93f6..1db3a7bbe 100644 ---
> >>>>  a/doc/guides/contributing/coding_style.rst +++
> >>>>  b/doc/guides/contributing/coding_style.rst @@ -21,7 +21,7 @@ The
> >>>>  rules and guidelines given in this document cannot cover every
> >>>>  situation, so * In the case of creating new files, the style should
> >>>>  be consistent within each file in a given directory or module.  *
> >>>>  The primary reason for coding standards is to increase code
> >>>>  readability and comprehensibility, therefore always use whatever
> >>>>  option
> >> will make the code easiest to read.
> >>>>
> >>>> -Line length is recommended to be not more than 80 characters,
> >>>> including comments.  +Line length is recommended to be not more than
> >>>> 100 characters, including comments.  [Tab stop size should be
> >>>> assumed to be 8-characters wide].
> >>>>
> >>>>  .. note::
> >>>
> >>> I would even support going to 120 characters.
> >>>
> >>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> >>
> >> I think 100 is enough.
> >>
> >> In my case, I have a 1080p 24" monitor, and with two terminals
> >> side-by-side 100 characters just fits inside each vim window. Going to
> >> 120 would be fine for single terminal at a time, but I would find
> >> awkward for e.g.  side-by-side diff comparison in meld etc.
> > 
> > My preference would be to keep things as it is - 80 chars per line.
> > Having multiple different formatting styles in one source file looks
> > really awkward and make it hard to follow.
> 
> +1
> 
I wouldn't personally consider increasing the max line length as a style
change, but even if you consider it such I'd worry about rejecting style
changes on the basis that it may be different to what is there before. That
logic means that we can never, ever change any element of DPDK coding style.

I can see the issue with changes that require us to rework the style of
code in order to comply with the new style, but changing the max length
from 80 to 100 does not make 80-char lines incorrect and needing changes.

Regards,
/Bruce
Jerin Jacob June 10, 2020, 5:22 a.m. UTC | #7 
On Tue, Jun 9, 2020 at 7:27 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, Jun 09, 2020 at 04:40:28PM +0300, Andrew Rybchenko wrote:
> > On 6/9/20 1:00 PM, Ananyev, Konstantin wrote:
> > >
> > >>
> > >> On Mon, Jun 08, 2020 at 12:17:23PM -0700, Stephen Hemminger wrote:
> > >>> On Mon,  8 Jun 2020 17:46:40 +0100 Bruce Richardson
> > >>> <bruce.richardson@intel.com> wrote:
> > >>>
> > >>>> Rather than continuing to recommend an 80-char line limit, let's
> > >>>> take a hint from the Linux kernel[1] and aim for an 100-char
> > >>>> recommended limit instead.
> > >>>>
> > >>>> [1]
> > >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> > >>>>
> > >>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
> > >>>> devtools/checkpatches.sh                 | 2 +-
> > >>>> doc/guides/contributing/coding_style.rst | 2 +- 2 files changed, 2
> > >>>> insertions(+), 2 deletions(-)
> > >>>>
> > >>>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> > >>>> index 158087f1c..4970ed830 100755 --- a/devtools/checkpatches.sh +++
> > >>>> b/devtools/checkpatches.sh @@ -15,7 +15,7 @@
> > >>>> VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
> > >>>> # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL
> > >>>> to a valid path # to a dictionary.txt file if dictionary.txt is not
> > >>>> in the default location.
> > >>>> codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
> > >>>> -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
> > >>>> +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
> > >>>>
> > >>>>  # override default Linux options options="--no-tree" diff --git
> > >>>>  a/doc/guides/contributing/coding_style.rst
> > >>>>  b/doc/guides/contributing/coding_style.rst index
> > >>>>  4efde93f6..1db3a7bbe 100644 ---
> > >>>>  a/doc/guides/contributing/coding_style.rst +++
> > >>>>  b/doc/guides/contributing/coding_style.rst @@ -21,7 +21,7 @@ The
> > >>>>  rules and guidelines given in this document cannot cover every
> > >>>>  situation, so * In the case of creating new files, the style should
> > >>>>  be consistent within each file in a given directory or module.  *
> > >>>>  The primary reason for coding standards is to increase code
> > >>>>  readability and comprehensibility, therefore always use whatever
> > >>>>  option
> > >> will make the code easiest to read.
> > >>>>
> > >>>> -Line length is recommended to be not more than 80 characters,
> > >>>> including comments.  +Line length is recommended to be not more than
> > >>>> 100 characters, including comments.  [Tab stop size should be
> > >>>> assumed to be 8-characters wide].
> > >>>>
> > >>>>  .. note::
> > >>>
> > >>> I would even support going to 120 characters.
> > >>>
> > >>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> > >>
> > >> I think 100 is enough.
> > >>
> > >> In my case, I have a 1080p 24" monitor, and with two terminals
> > >> side-by-side 100 characters just fits inside each vim window. Going to
> > >> 120 would be fine for single terminal at a time, but I would find
> > >> awkward for e.g.  side-by-side diff comparison in meld etc.
> > >
> > > My preference would be to keep things as it is - 80 chars per line.
> > > Having multiple different formatting styles in one source file looks
> > > really awkward and make it hard to follow.
> >
> > +1
> >
> I wouldn't personally consider increasing the max line length as a style
> change, but even if you consider it such I'd worry about rejecting style
> changes on the basis that it may be different to what is there before. That
> logic means that we can never, ever change any element of DPDK coding style.
>
> I can see the issue with changes that require us to rework the style of
> code in order to comply with the new style, but changing the max length
> from 80 to 100 does not make 80-char lines incorrect and needing changes.

Another point is: Other projects derived from the Linux kernel coding
standard also
getting migrated to the new coding standard. This change would be useful for:
a) People works on multiple Linux coding standard derived projects
b) Some of the code such as 'base' and 'common' code for HW drivers
are shared between multiple projects.
Such code needs adjustment/change when pulling to the DPDK code base
it it still follows 80 chars per line.


>
> Regards,
> /Bruce
Andrew Rybchenko June 10, 2020, 8:27 a.m. UTC | #8 
On 6/10/20 8:22 AM, Jerin Jacob wrote:
> On Tue, Jun 9, 2020 at 7:27 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
>>
>> On Tue, Jun 09, 2020 at 04:40:28PM +0300, Andrew Rybchenko wrote:
>>> On 6/9/20 1:00 PM, Ananyev, Konstantin wrote:
>>>>
>>>>>
>>>>> On Mon, Jun 08, 2020 at 12:17:23PM -0700, Stephen Hemminger wrote:
>>>>>> On Mon,  8 Jun 2020 17:46:40 +0100 Bruce Richardson
>>>>>> <bruce.richardson@intel.com> wrote:
>>>>>>
>>>>>>> Rather than continuing to recommend an 80-char line limit, let's
>>>>>>> take a hint from the Linux kernel[1] and aim for an 100-char
>>>>>>> recommended limit instead.
>>>>>>>
>>>>>>> [1]
>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
>>>>>>>
>>>>>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
>>>>>>> devtools/checkpatches.sh                 | 2 +-
>>>>>>> doc/guides/contributing/coding_style.rst | 2 +- 2 files changed, 2
>>>>>>> insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
>>>>>>> index 158087f1c..4970ed830 100755 --- a/devtools/checkpatches.sh +++
>>>>>>> b/devtools/checkpatches.sh @@ -15,7 +15,7 @@
>>>>>>> VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
>>>>>>> # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL
>>>>>>> to a valid path # to a dictionary.txt file if dictionary.txt is not
>>>>>>> in the default location.
>>>>>>> codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
>>>>>>> -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
>>>>>>> +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
>>>>>>>
>>>>>>>  # override default Linux options options="--no-tree" diff --git
>>>>>>>  a/doc/guides/contributing/coding_style.rst
>>>>>>>  b/doc/guides/contributing/coding_style.rst index
>>>>>>>  4efde93f6..1db3a7bbe 100644 ---
>>>>>>>  a/doc/guides/contributing/coding_style.rst +++
>>>>>>>  b/doc/guides/contributing/coding_style.rst @@ -21,7 +21,7 @@ The
>>>>>>>  rules and guidelines given in this document cannot cover every
>>>>>>>  situation, so * In the case of creating new files, the style should
>>>>>>>  be consistent within each file in a given directory or module.  *
>>>>>>>  The primary reason for coding standards is to increase code
>>>>>>>  readability and comprehensibility, therefore always use whatever
>>>>>>>  option
>>>>> will make the code easiest to read.
>>>>>>>
>>>>>>> -Line length is recommended to be not more than 80 characters,
>>>>>>> including comments.  +Line length is recommended to be not more than
>>>>>>> 100 characters, including comments.  [Tab stop size should be
>>>>>>> assumed to be 8-characters wide].
>>>>>>>
>>>>>>>  .. note::
>>>>>>
>>>>>> I would even support going to 120 characters.
>>>>>>
>>>>>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>>>>>
>>>>> I think 100 is enough.
>>>>>
>>>>> In my case, I have a 1080p 24" monitor, and with two terminals
>>>>> side-by-side 100 characters just fits inside each vim window. Going to
>>>>> 120 would be fine for single terminal at a time, but I would find
>>>>> awkward for e.g.  side-by-side diff comparison in meld etc.
>>>>
>>>> My preference would be to keep things as it is - 80 chars per line.
>>>> Having multiple different formatting styles in one source file looks
>>>> really awkward and make it hard to follow.
>>>
>>> +1
>>>
>> I wouldn't personally consider increasing the max line length as a style
>> change, but even if you consider it such I'd worry about rejecting style
>> changes on the basis that it may be different to what is there before. That
>> logic means that we can never, ever change any element of DPDK coding style.
>>
>> I can see the issue with changes that require us to rework the style of
>> code in order to comply with the new style, but changing the max length
>> from 80 to 100 does not make 80-char lines incorrect and needing changes.
> 
> Another point is: Other projects derived from the Linux kernel coding
> standard also
> getting migrated to the new coding standard. This change would be useful for:
> a) People works on multiple Linux coding standard derived projects

Valid point, but not really strong.
I think that .editorconfig solves it.

> b) Some of the code such as 'base' and 'common' code for HW drivers
> are shared between multiple projects.
> Such code needs adjustment/change when pulling to the DPDK code base
> it it still follows 80 chars per line.

Base and common code are not required to follow DPDK coding
style even now.
Jerin Jacob June 10, 2020, 8:47 a.m. UTC | #9 
On Wed, Jun 10, 2020 at 1:57 PM Andrew Rybchenko
<arybchenko@solarflare.com> wrote:
>
> On 6/10/20 8:22 AM, Jerin Jacob wrote:
> > On Tue, Jun 9, 2020 at 7:27 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> >>
> >> On Tue, Jun 09, 2020 at 04:40:28PM +0300, Andrew Rybchenko wrote:
> >>> On 6/9/20 1:00 PM, Ananyev, Konstantin wrote:
> >>>>
> >>>>>
> >>>>> On Mon, Jun 08, 2020 at 12:17:23PM -0700, Stephen Hemminger wrote:
> >>>>>> On Mon,  8 Jun 2020 17:46:40 +0100 Bruce Richardson
> >>>>>> <bruce.richardson@intel.com> wrote:
> >>>>>>
> >>>>>>> Rather than continuing to recommend an 80-char line limit, let's
> >>>>>>> take a hint from the Linux kernel[1] and aim for an 100-char
> >>>>>>> recommended limit instead.
> >>>>>>>
> >>>>>>> [1]
> >>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> >>>>>>>
> >>>>>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
> >>>>>>> devtools/checkpatches.sh                 | 2 +-
> >>>>>>> doc/guides/contributing/coding_style.rst | 2 +- 2 files changed, 2
> >>>>>>> insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> >>>>>>> index 158087f1c..4970ed830 100755 --- a/devtools/checkpatches.sh +++
> >>>>>>> b/devtools/checkpatches.sh @@ -15,7 +15,7 @@
> >>>>>>> VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
> >>>>>>> # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL
> >>>>>>> to a valid path # to a dictionary.txt file if dictionary.txt is not
> >>>>>>> in the default location.
> >>>>>>> codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
> >>>>>>> -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
> >>>>>>> +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
> >>>>>>>
> >>>>>>>  # override default Linux options options="--no-tree" diff --git
> >>>>>>>  a/doc/guides/contributing/coding_style.rst
> >>>>>>>  b/doc/guides/contributing/coding_style.rst index
> >>>>>>>  4efde93f6..1db3a7bbe 100644 ---
> >>>>>>>  a/doc/guides/contributing/coding_style.rst +++
> >>>>>>>  b/doc/guides/contributing/coding_style.rst @@ -21,7 +21,7 @@ The
> >>>>>>>  rules and guidelines given in this document cannot cover every
> >>>>>>>  situation, so * In the case of creating new files, the style should
> >>>>>>>  be consistent within each file in a given directory or module.  *
> >>>>>>>  The primary reason for coding standards is to increase code
> >>>>>>>  readability and comprehensibility, therefore always use whatever
> >>>>>>>  option
> >>>>> will make the code easiest to read.
> >>>>>>>
> >>>>>>> -Line length is recommended to be not more than 80 characters,
> >>>>>>> including comments.  +Line length is recommended to be not more than
> >>>>>>> 100 characters, including comments.  [Tab stop size should be
> >>>>>>> assumed to be 8-characters wide].
> >>>>>>>
> >>>>>>>  .. note::
> >>>>>>
> >>>>>> I would even support going to 120 characters.
> >>>>>>
> >>>>>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> >>>>>
> >>>>> I think 100 is enough.
> >>>>>
> >>>>> In my case, I have a 1080p 24" monitor, and with two terminals
> >>>>> side-by-side 100 characters just fits inside each vim window. Going to
> >>>>> 120 would be fine for single terminal at a time, but I would find
> >>>>> awkward for e.g.  side-by-side diff comparison in meld etc.
> >>>>
> >>>> My preference would be to keep things as it is - 80 chars per line.
> >>>> Having multiple different formatting styles in one source file looks
> >>>> really awkward and make it hard to follow.
> >>>
> >>> +1
> >>>
> >> I wouldn't personally consider increasing the max line length as a style
> >> change, but even if you consider it such I'd worry about rejecting style
> >> changes on the basis that it may be different to what is there before. That
> >> logic means that we can never, ever change any element of DPDK coding style.
> >>
> >> I can see the issue with changes that require us to rework the style of
> >> code in order to comply with the new style, but changing the max length
> >> from 80 to 100 does not make 80-char lines incorrect and needing changes.
> >
> > Another point is: Other projects derived from the Linux kernel coding
> > standard also
> > getting migrated to the new coding standard. This change would be useful for:
> > a) People works on multiple Linux coding standard derived projects
>
> Valid point, but not really strong.
> I think that .editorconfig solves it.

Yes, For adding the code. I meaning, Viewing the code there will be a disparity.

>
> > b) Some of the code such as 'base' and 'common' code for HW drivers
> > are shared between multiple projects.
> > Such code needs adjustment/change when pulling to the DPDK code base
> > it it still follows 80 chars per line.
>
> Base and common code are not required to follow DPDK coding
> style even now.

I see, I dont think it is expressed in devtools/checkpatches.sh. I.e
CI tools still flag as checkpatch issues.

Coming to original concern:(code disparity with existing code)
Another option is, It is possible to change existing code to 100 lines
with clang-format in an automatic fashion. But it will have a lot of
changes.
        "C_Cpp.clang_format_style": "{ BasedOnStyle: LLVM,
IndentWidth: 8, TabWidth: 8, UseTab: Always,
AllowShortIfStatementsOnASingleLine: false, IndentCaseLabels: false,
ColumnLimit: 100, AllowShortFunctionsOnASingleLine: false,
AlwaysBreakAfterReturnType: AllDefinitions, ColumnLimit: 100,
ConstructorInitializerAllOnOneLineOrOnePerLine: true,
ConstructorInitializerIndentWidth: 8, ContinuationIndentWidth: 8,
BreakBeforeBraces: Linux, AllowShortBlocksOnASingleLine: false,
AlignConsecutiveAssignments: false, AlignEscapedNewlines: Right,
AlignConsecutiveMacros : true, MaxEmptyLinesToKeep : 1,
Cpp11BracedListStyle : true, AlignTrailingComments : true,
ForEachMacros: ['TAILQ_FOREACH_SAFE', STAILQ_FOREACH',
'rte_graph_foreach_node', 'TAILQ_FOREACH', 'RTE_ETH_FOREACH_DEV']}",
Andrew Rybchenko June 10, 2020, 9:28 a.m. UTC | #10 
On 6/10/20 11:47 AM, Jerin Jacob wrote:
> On Wed, Jun 10, 2020 at 1:57 PM Andrew Rybchenko
> <arybchenko@solarflare.com> wrote:
>>
>> On 6/10/20 8:22 AM, Jerin Jacob wrote:
>>> On Tue, Jun 9, 2020 at 7:27 PM Bruce Richardson
>>> <bruce.richardson@intel.com> wrote:
>>>>
>>>> On Tue, Jun 09, 2020 at 04:40:28PM +0300, Andrew Rybchenko wrote:
>>>>> On 6/9/20 1:00 PM, Ananyev, Konstantin wrote:
>>>>>>
>>>>>>>
>>>>>>> On Mon, Jun 08, 2020 at 12:17:23PM -0700, Stephen Hemminger wrote:
>>>>>>>> On Mon,  8 Jun 2020 17:46:40 +0100 Bruce Richardson
>>>>>>>> <bruce.richardson@intel.com> wrote:
>>>>>>>>
>>>>>>>>> Rather than continuing to recommend an 80-char line limit, let's
>>>>>>>>> take a hint from the Linux kernel[1] and aim for an 100-char
>>>>>>>>> recommended limit instead.
>>>>>>>>>
>>>>>>>>> [1]
>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
>>>>>>>>>
>>>>>>>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
>>>>>>>>> devtools/checkpatches.sh                 | 2 +-
>>>>>>>>> doc/guides/contributing/coding_style.rst | 2 +- 2 files changed, 2
>>>>>>>>> insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
>>>>>>>>> index 158087f1c..4970ed830 100755 --- a/devtools/checkpatches.sh +++
>>>>>>>>> b/devtools/checkpatches.sh @@ -15,7 +15,7 @@
>>>>>>>>> VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
>>>>>>>>> # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL
>>>>>>>>> to a valid path # to a dictionary.txt file if dictionary.txt is not
>>>>>>>>> in the default location.
>>>>>>>>> codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
>>>>>>>>> -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
>>>>>>>>> +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
>>>>>>>>>
>>>>>>>>>  # override default Linux options options="--no-tree" diff --git
>>>>>>>>>  a/doc/guides/contributing/coding_style.rst
>>>>>>>>>  b/doc/guides/contributing/coding_style.rst index
>>>>>>>>>  4efde93f6..1db3a7bbe 100644 ---
>>>>>>>>>  a/doc/guides/contributing/coding_style.rst +++
>>>>>>>>>  b/doc/guides/contributing/coding_style.rst @@ -21,7 +21,7 @@ The
>>>>>>>>>  rules and guidelines given in this document cannot cover every
>>>>>>>>>  situation, so * In the case of creating new files, the style should
>>>>>>>>>  be consistent within each file in a given directory or module.  *
>>>>>>>>>  The primary reason for coding standards is to increase code
>>>>>>>>>  readability and comprehensibility, therefore always use whatever
>>>>>>>>>  option
>>>>>>> will make the code easiest to read.
>>>>>>>>>
>>>>>>>>> -Line length is recommended to be not more than 80 characters,
>>>>>>>>> including comments.  +Line length is recommended to be not more than
>>>>>>>>> 100 characters, including comments.  [Tab stop size should be
>>>>>>>>> assumed to be 8-characters wide].
>>>>>>>>>
>>>>>>>>>  .. note::
>>>>>>>>
>>>>>>>> I would even support going to 120 characters.
>>>>>>>>
>>>>>>>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>>>>>>>
>>>>>>> I think 100 is enough.
>>>>>>>
>>>>>>> In my case, I have a 1080p 24" monitor, and with two terminals
>>>>>>> side-by-side 100 characters just fits inside each vim window. Going to
>>>>>>> 120 would be fine for single terminal at a time, but I would find
>>>>>>> awkward for e.g.  side-by-side diff comparison in meld etc.
>>>>>>
>>>>>> My preference would be to keep things as it is - 80 chars per line.
>>>>>> Having multiple different formatting styles in one source file looks
>>>>>> really awkward and make it hard to follow.
>>>>>
>>>>> +1
>>>>>
>>>> I wouldn't personally consider increasing the max line length as a style
>>>> change, but even if you consider it such I'd worry about rejecting style
>>>> changes on the basis that it may be different to what is there before. That
>>>> logic means that we can never, ever change any element of DPDK coding style.
>>>>
>>>> I can see the issue with changes that require us to rework the style of
>>>> code in order to comply with the new style, but changing the max length
>>>> from 80 to 100 does not make 80-char lines incorrect and needing changes.
>>>
>>> Another point is: Other projects derived from the Linux kernel coding
>>> standard also
>>> getting migrated to the new coding standard. This change would be useful for:
>>> a) People works on multiple Linux coding standard derived projects
>>
>> Valid point, but not really strong.
>> I think that .editorconfig solves it.
> 
> Yes, For adding the code. I meaning, Viewing the code there will be a disparity.

I hope you're not suggesting to reformat all existing code.
Otherwise the disparity will be there for a long-long time
anyway.

>>
>>> b) Some of the code such as 'base' and 'common' code for HW drivers
>>> are shared between multiple projects.
>>> Such code needs adjustment/change when pulling to the DPDK code base
>>> it it still follows 80 chars per line.
>>
>> Base and common code are not required to follow DPDK coding
>> style even now.
> 
> I see, I dont think it is expressed in devtools/checkpatches.sh. I.e
> CI tools still flag as checkpatch issues.

Yes, it is an area which should be improved.

> Coming to original concern:(code disparity with existing code)
> Another option is, It is possible to change existing code to 100 lines
> with clang-format in an automatic fashion. But it will have a lot of
> changes.
>         "C_Cpp.clang_format_style": "{ BasedOnStyle: LLVM,
> IndentWidth: 8, TabWidth: 8, UseTab: Always,
> AllowShortIfStatementsOnASingleLine: false, IndentCaseLabels: false,
> ColumnLimit: 100, AllowShortFunctionsOnASingleLine: false,
> AlwaysBreakAfterReturnType: AllDefinitions, ColumnLimit: 100,
> ConstructorInitializerAllOnOneLineOrOnePerLine: true,
> ConstructorInitializerIndentWidth: 8, ContinuationIndentWidth: 8,
> BreakBeforeBraces: Linux, AllowShortBlocksOnASingleLine: false,
> AlignConsecutiveAssignments: false, AlignEscapedNewlines: Right,
> AlignConsecutiveMacros : true, MaxEmptyLinesToKeep : 1,
> Cpp11BracedListStyle : true, AlignTrailingComments : true,
> ForEachMacros: ['TAILQ_FOREACH_SAFE', STAILQ_FOREACH',
> 'rte_graph_foreach_node', 'TAILQ_FOREACH', 'RTE_ETH_FOREACH_DEV']}",

No, no, no. Please, no. It will complicate backporting a lot,
it will break (over-complicate) git blame.
(I hope it was suggested just to be sure that it will not be
done).
Bruce Richardson June 10, 2020, 9:59 a.m. UTC | #11 
On Wed, Jun 10, 2020 at 12:28:53PM +0300, Andrew Rybchenko wrote:
> On 6/10/20 11:47 AM, Jerin Jacob wrote:
> > On Wed, Jun 10, 2020 at 1:57 PM Andrew Rybchenko
> > <arybchenko@solarflare.com> wrote:
> >>
> >> On 6/10/20 8:22 AM, Jerin Jacob wrote:
> >>> On Tue, Jun 9, 2020 at 7:27 PM Bruce Richardson
> >>> <bruce.richardson@intel.com> wrote:
> >>>>
> >>>> On Tue, Jun 09, 2020 at 04:40:28PM +0300, Andrew Rybchenko wrote:
> >>>>> On 6/9/20 1:00 PM, Ananyev, Konstantin wrote:
> >>>>>>
> >>>>>>>
> >>>>>>> On Mon, Jun 08, 2020 at 12:17:23PM -0700, Stephen Hemminger wrote:
> >>>>>>>> On Mon,  8 Jun 2020 17:46:40 +0100 Bruce Richardson
> >>>>>>>> <bruce.richardson@intel.com> wrote:
> >>>>>>>>
> >>>>>>>>> Rather than continuing to recommend an 80-char line limit, let's
> >>>>>>>>> take a hint from the Linux kernel[1] and aim for an 100-char
> >>>>>>>>> recommended limit instead.
> >>>>>>>>>
> >>>>>>>>> [1]
> >>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
> >>>>>>>>> devtools/checkpatches.sh                 | 2 +-
> >>>>>>>>> doc/guides/contributing/coding_style.rst | 2 +- 2 files changed, 2
> >>>>>>>>> insertions(+), 2 deletions(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> >>>>>>>>> index 158087f1c..4970ed830 100755 --- a/devtools/checkpatches.sh +++
> >>>>>>>>> b/devtools/checkpatches.sh @@ -15,7 +15,7 @@
> >>>>>>>>> VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
> >>>>>>>>> # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL
> >>>>>>>>> to a valid path # to a dictionary.txt file if dictionary.txt is not
> >>>>>>>>> in the default location.
> >>>>>>>>> codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
> >>>>>>>>> -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
> >>>>>>>>> +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
> >>>>>>>>>
> >>>>>>>>>  # override default Linux options options="--no-tree" diff --git
> >>>>>>>>>  a/doc/guides/contributing/coding_style.rst
> >>>>>>>>>  b/doc/guides/contributing/coding_style.rst index
> >>>>>>>>>  4efde93f6..1db3a7bbe 100644 ---
> >>>>>>>>>  a/doc/guides/contributing/coding_style.rst +++
> >>>>>>>>>  b/doc/guides/contributing/coding_style.rst @@ -21,7 +21,7 @@ The
> >>>>>>>>>  rules and guidelines given in this document cannot cover every
> >>>>>>>>>  situation, so * In the case of creating new files, the style should
> >>>>>>>>>  be consistent within each file in a given directory or module.  *
> >>>>>>>>>  The primary reason for coding standards is to increase code
> >>>>>>>>>  readability and comprehensibility, therefore always use whatever
> >>>>>>>>>  option
> >>>>>>> will make the code easiest to read.
> >>>>>>>>>
> >>>>>>>>> -Line length is recommended to be not more than 80 characters,
> >>>>>>>>> including comments.  +Line length is recommended to be not more than
> >>>>>>>>> 100 characters, including comments.  [Tab stop size should be
> >>>>>>>>> assumed to be 8-characters wide].
> >>>>>>>>>
> >>>>>>>>>  .. note::
> >>>>>>>>
> >>>>>>>> I would even support going to 120 characters.
> >>>>>>>>
> >>>>>>>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> >>>>>>>
> >>>>>>> I think 100 is enough.
> >>>>>>>
> >>>>>>> In my case, I have a 1080p 24" monitor, and with two terminals
> >>>>>>> side-by-side 100 characters just fits inside each vim window. Going to
> >>>>>>> 120 would be fine for single terminal at a time, but I would find
> >>>>>>> awkward for e.g.  side-by-side diff comparison in meld etc.
> >>>>>>
> >>>>>> My preference would be to keep things as it is - 80 chars per line.
> >>>>>> Having multiple different formatting styles in one source file looks
> >>>>>> really awkward and make it hard to follow.
> >>>>>
> >>>>> +1
> >>>>>
> >>>> I wouldn't personally consider increasing the max line length as a style
> >>>> change, but even if you consider it such I'd worry about rejecting style
> >>>> changes on the basis that it may be different to what is there before. That
> >>>> logic means that we can never, ever change any element of DPDK coding style.
> >>>>
> >>>> I can see the issue with changes that require us to rework the style of
> >>>> code in order to comply with the new style, but changing the max length
> >>>> from 80 to 100 does not make 80-char lines incorrect and needing changes.
> >>>
> >>> Another point is: Other projects derived from the Linux kernel coding
> >>> standard also
> >>> getting migrated to the new coding standard. This change would be useful for:
> >>> a) People works on multiple Linux coding standard derived projects
> >>
> >> Valid point, but not really strong.
> >> I think that .editorconfig solves it.
> > 
> > Yes, For adding the code. I meaning, Viewing the code there will be a disparity.
> 
> I hope you're not suggesting to reformat all existing code.
> Otherwise the disparity will be there for a long-long time
> anyway.
> 
> >>
> >>> b) Some of the code such as 'base' and 'common' code for HW drivers
> >>> are shared between multiple projects.
> >>> Such code needs adjustment/change when pulling to the DPDK code base
> >>> it it still follows 80 chars per line.
> >>
> >> Base and common code are not required to follow DPDK coding
> >> style even now.
> > 
> > I see, I dont think it is expressed in devtools/checkpatches.sh. I.e
> > CI tools still flag as checkpatch issues.
> 
> Yes, it is an area which should be improved.
> 
> > Coming to original concern:(code disparity with existing code)
> > Another option is, It is possible to change existing code to 100 lines
> > with clang-format in an automatic fashion. But it will have a lot of
> > changes.
> >         "C_Cpp.clang_format_style": "{ BasedOnStyle: LLVM,
> > IndentWidth: 8, TabWidth: 8, UseTab: Always,
> > AllowShortIfStatementsOnASingleLine: false, IndentCaseLabels: false,
> > ColumnLimit: 100, AllowShortFunctionsOnASingleLine: false,
> > AlwaysBreakAfterReturnType: AllDefinitions, ColumnLimit: 100,
> > ConstructorInitializerAllOnOneLineOrOnePerLine: true,
> > ConstructorInitializerIndentWidth: 8, ContinuationIndentWidth: 8,
> > BreakBeforeBraces: Linux, AllowShortBlocksOnASingleLine: false,
> > AlignConsecutiveAssignments: false, AlignEscapedNewlines: Right,
> > AlignConsecutiveMacros : true, MaxEmptyLinesToKeep : 1,
> > Cpp11BracedListStyle : true, AlignTrailingComments : true,
> > ForEachMacros: ['TAILQ_FOREACH_SAFE', STAILQ_FOREACH',
> > 'rte_graph_foreach_node', 'TAILQ_FOREACH', 'RTE_ETH_FOREACH_DEV']}",
> 
> No, no, no. Please, no. It will complicate backporting a lot,
> it will break (over-complicate) git blame.
> (I hope it was suggested just to be sure that it will not be
> done).

+1 for this. I don't think we can ever take any style changes into DPDK
which require us to reformat all our existing code.

Thankfully, as I've stated before, this is not such a change, since code
which is under 80 chars long is also under 100 chars long so no issue. :-)
Thomas Monjalon Aug. 7, 2020, 12:29 a.m. UTC | #12 
08/06/2020 18:46, Bruce Richardson:
> Rather than continuing to recommend an 80-char line limit, let's take a hint
> from the Linux kernel[1] and aim for an 100-char recommended limit instead.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> -length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
> +length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}

This is my personnal settings for years.

[...]
> -Line length is recommended to be not more than 80 characters, including comments.
> +Line length is recommended to be not more than 100 characters, including comments.

I think lines of 80 characters are reasonnable,
but I would not reject a patch because few lines are between 80 and 100.

I would reword the doc to reflect that flexibility:
"
Line length is recommended to be not more than 80 characters, including comments.
The hard limit is 100 characters.
"
diff mbox series

Patch

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 158087f1c..4970ed830 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -15,7 +15,7 @@  VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
 # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL to a valid path
 # to a dictionary.txt file if dictionary.txt is not in the default location.
 codespell=${DPDK_CHECKPATCH_CODESPELL:-enable}
-length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
+length=${DPDK_CHECKPATCH_LINE_LENGTH:-100}
 
 # override default Linux options
 options="--no-tree"
diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
index 4efde93f6..1db3a7bbe 100644
--- a/doc/guides/contributing/coding_style.rst
+++ b/doc/guides/contributing/coding_style.rst
@@ -21,7 +21,7 @@  The rules and guidelines given in this document cannot cover every situation, so
 * In the case of creating new files, the style should be consistent within each file in a given directory or module.
 * The primary reason for coding standards is to increase code readability and comprehensibility, therefore always use whatever option will make the code easiest to read.
 
-Line length is recommended to be not more than 80 characters, including comments.
+Line length is recommended to be not more than 100 characters, including comments.
 [Tab stop size should be assumed to be 8-characters wide].
 
 .. note::