DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
@ 2020-06-08 16:46 Bruce Richardson
  2020-06-08 19:17 ` Stephen Hemminger
  2020-08-07  0:29 ` Thomas Monjalon
  0 siblings, 2 replies; 13+ messages in thread
From: Bruce Richardson @ 2020-06-08 16:46 UTC (permalink / raw)
  To: dev; +Cc: 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>
---
 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::
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-08 16:46 [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100 Bruce Richardson
@ 2020-06-08 19:17 ` Stephen Hemminger
  2020-06-09  4:42   ` Jerin Jacob
  2020-06-09  9:29   ` Bruce Richardson
  2020-08-07  0:29 ` Thomas Monjalon
  1 sibling, 2 replies; 13+ messages in thread
From: Stephen Hemminger @ 2020-06-08 19:17 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

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>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-08 19:17 ` Stephen Hemminger
@ 2020-06-09  4:42   ` Jerin Jacob
  2020-06-09  9:29   ` Bruce Richardson
  1 sibling, 0 replies; 13+ messages in thread
From: Jerin Jacob @ 2020-06-09  4:42 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Bruce Richardson, dpdk-dev

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>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-08 19:17 ` Stephen Hemminger
  2020-06-09  4:42   ` Jerin Jacob
@ 2020-06-09  9:29   ` Bruce Richardson
  2020-06-09 10:00     ` Ananyev, Konstantin
  1 sibling, 1 reply; 13+ messages in thread
From: Bruce Richardson @ 2020-06-09  9:29 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-09  9:29   ` Bruce Richardson
@ 2020-06-09 10:00     ` Ananyev, Konstantin
  2020-06-09 13:40       ` Andrew Rybchenko
  0 siblings, 1 reply; 13+ messages in thread
From: Ananyev, Konstantin @ 2020-06-09 10:00 UTC (permalink / raw)
  To: Richardson, Bruce, Stephen Hemminger; +Cc: dev


> 
> 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.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-09 10:00     ` Ananyev, Konstantin
@ 2020-06-09 13:40       ` Andrew Rybchenko
  2020-06-09 13:57         ` Bruce Richardson
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Rybchenko @ 2020-06-09 13:40 UTC (permalink / raw)
  To: Ananyev, Konstantin, Richardson, Bruce, Stephen Hemminger; +Cc: dev

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-09 13:40       ` Andrew Rybchenko
@ 2020-06-09 13:57         ` Bruce Richardson
  2020-06-10  5:22           ` Jerin Jacob
  0 siblings, 1 reply; 13+ messages in thread
From: Bruce Richardson @ 2020-06-09 13:57 UTC (permalink / raw)
  To: Andrew Rybchenko; +Cc: Ananyev, Konstantin, Stephen Hemminger, dev

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-09 13:57         ` Bruce Richardson
@ 2020-06-10  5:22           ` Jerin Jacob
  2020-06-10  8:27             ` Andrew Rybchenko
  0 siblings, 1 reply; 13+ messages in thread
From: Jerin Jacob @ 2020-06-10  5:22 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Andrew Rybchenko, Ananyev, Konstantin, Stephen Hemminger, dev

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-10  5:22           ` Jerin Jacob
@ 2020-06-10  8:27             ` Andrew Rybchenko
  2020-06-10  8:47               ` Jerin Jacob
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Rybchenko @ 2020-06-10  8:27 UTC (permalink / raw)
  To: Jerin Jacob, Bruce Richardson; +Cc: Ananyev, Konstantin, Stephen Hemminger, dev

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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-10  8:27             ` Andrew Rybchenko
@ 2020-06-10  8:47               ` Jerin Jacob
  2020-06-10  9:28                 ` Andrew Rybchenko
  0 siblings, 1 reply; 13+ messages in thread
From: Jerin Jacob @ 2020-06-10  8:47 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: Bruce Richardson, Ananyev, Konstantin, Stephen Hemminger, dev

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']}",

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-10  8:47               ` Jerin Jacob
@ 2020-06-10  9:28                 ` Andrew Rybchenko
  2020-06-10  9:59                   ` Bruce Richardson
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Rybchenko @ 2020-06-10  9:28 UTC (permalink / raw)
  To: Jerin Jacob; +Cc: Bruce Richardson, Ananyev, Konstantin, Stephen Hemminger, dev

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).

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-10  9:28                 ` Andrew Rybchenko
@ 2020-06-10  9:59                   ` Bruce Richardson
  0 siblings, 0 replies; 13+ messages in thread
From: Bruce Richardson @ 2020-06-10  9:59 UTC (permalink / raw)
  To: Andrew Rybchenko; +Cc: Jerin Jacob, Ananyev, Konstantin, Stephen Hemminger, dev

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. :-)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
  2020-06-08 16:46 [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100 Bruce Richardson
  2020-06-08 19:17 ` Stephen Hemminger
@ 2020-08-07  0:29 ` Thomas Monjalon
  1 sibling, 0 replies; 13+ messages in thread
From: Thomas Monjalon @ 2020-08-07  0:29 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: dev, stephen, jerinj, arybchenko, david.marchand, ferruh.yigit,
	olivier.matz, akhil.goyal

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.
"



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-08-07  0:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-08 16:46 [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100 Bruce Richardson
2020-06-08 19:17 ` Stephen Hemminger
2020-06-09  4:42   ` Jerin Jacob
2020-06-09  9:29   ` Bruce Richardson
2020-06-09 10:00     ` Ananyev, Konstantin
2020-06-09 13:40       ` Andrew Rybchenko
2020-06-09 13:57         ` Bruce Richardson
2020-06-10  5:22           ` Jerin Jacob
2020-06-10  8:27             ` Andrew Rybchenko
2020-06-10  8:47               ` Jerin Jacob
2020-06-10  9:28                 ` Andrew Rybchenko
2020-06-10  9:59                   ` Bruce Richardson
2020-08-07  0:29 ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).