From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] checkpatch: re-enable warnings about split long strings
Date: Mon, 2 Oct 2017 13:53:17 +0200 [thread overview]
Message-ID: <20171002115317.GJ3871@6wind.com> (raw)
In-Reply-To: <20170929153749.9806-1-stephen@networkplumber.org>
Hi Stephen,
On Fri, Sep 29, 2017 at 08:37:49AM -0700, Stephen Hemminger wrote:
> The Linux kernel style policy about strings is that strings should
> be always put on one line. This makes sense since a typical use
> case is for a user to type the error message into a search engine
> or grep, and it won't be found if split across lines.
> This patch just re-enables that check.
>
> Yes, lots of DPDK code now splits strings, that doesn't
> make it right.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
> devtools/checkpatches.sh | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index a56c41a301c0..3e6081dd673e 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -44,7 +44,6 @@ options="$options --show-types"
> options="$options --ignore=LINUX_VERSION_CODE,FILE_PATH_CHANGES,\
> VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
> PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,\
> -SPLIT_STRING,LONG_LINE_STRING,\
> LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
> NEW_TYPEDEFS,COMPARISON_TO_NULL"
I'm not sure, given that the main reason for splitting strings in the first
place is to avoid LONG_LINE_STRING warnings, I think we must choose between
the two options. If split strings are not allowed, then long lines must be.
Since checkpatches.sh is used by various automated scripts to complain
loudly about problems in submissions, the above change prevents maintainers
from writing long string at all (can't split and can't go past 80 columns).
As a result, they will be tempted to cripple their code with nasty
workarounds to shut up checkpatches.sh, we don't want that to happen.
Also I think the reasons stated by original commit cf75514c8e2e are still
relevant. My vote would be to keep things as is.
--
Adrien Mazarguil
6WIND
next prev parent reply other threads:[~2017-10-02 11:53 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-29 15:37 Stephen Hemminger
2017-10-02 10:01 ` Luca Boccassi
2017-10-02 11:53 ` Adrien Mazarguil [this message]
2017-10-02 13:46 ` Bruce Richardson
2017-10-02 16:21 ` Adrien Mazarguil
2017-10-03 10:38 ` Bruce Richardson
2017-10-03 10:56 ` Adrien Mazarguil
2019-03-28 19:02 ` Ferruh Yigit
2019-03-28 19:02 ` Ferruh Yigit
2019-03-28 19:03 ` Ferruh Yigit
2019-03-28 19:03 ` Ferruh Yigit
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=20171002115317.GJ3871@6wind.com \
--to=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).