DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	Stephen Hemminger <stephen@networkplumber.org>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH RFC] devtools: increase default line length to 100
Date: Tue, 9 Jun 2020 14:57:17 +0100
Message-ID: <20200609135717.GA1583@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <5710188e-4bba-c909-34cb-1bb67cbc3af0@solarflare.com>

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

  reply	other threads:[~2020-06-09 13:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-08 16:46 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 [this message]
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

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20200609135717.GA1583@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git