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