mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Markus Elfring <Markus.Elfring@web.de>
To: Wang YanQing <udknight@gmail.com>, Joe Perches <joe@perches.com>,
	Andy Whitcroft <apw@canonical.com>,
	kernel-janitors@vger.kernel.org
Cc: linux-kernel@vger.kernel.org,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Matteo Croce <mcroce@redhat.com>
Subject: Re: [PATCH v3] checkpatch: add dedicated checker for 'Fixes:' tag
Date: Tue, 28 Apr 2020 08:21:37 +0200	[thread overview]
Message-ID: <35069ae9-cdb4-4728-5e94-6d863dd2721c@web.de> (raw)
In-Reply-To: <20200428020223.GA28074@udknight>

> ...
> Do not split the tag across multiple
> lines, tags are exempt from the "wrap at 75 columns" rule in order to simplify
> parsing scripts
> ...

I suggest to reformat the quotation.

“…
Do not split the tag across multiple lines, tags are exempt from
the "wrap at 75 columns" rule in order to simplify parsing scripts.
…”


> And there is no 'Fixes:' tag format checker in checkpatch to check
> the commit id length too, so let's add dedicated checker to check
> these conditions for 'Fixes:' tag.

It seems that there are further challenges to consider for an imperative wording
in such a change description.

How do you think about the following wording variant?

   The script “checkpatch.pl” did not provide a check for the commit
   identification length. Thus add a dedicated check.


>  v2-v3

I would find a shorter version identification (without the arrow)
also sufficient.


>  1:No modification to GIT_COMMIT_ID checker.

Would you like to add a space character for the item enumeration?


>    I make a mistake previously, …

Would you like to use the word “made” here?


> +		    my $id = '0123456789ab';
> +		    my $orig_desc = "commit description";

How much do these variable initialisations matter?


> +			$lines = 0 if ($1 =~ /^\(\"(?:.*)\"\)$/i);

I wonder why you see a need to use a non-capturing group in such
a regular expression (when no alternatives were specified there).


> +				ERROR("FIXES_TAG",
> +					"Please use 'Fixes:' tag with commit description style '<12+ chars of sha1> (\"<title line>\")', and the title line doesn't across multiple lines - ie: '$fixes_tag_fmt'\n" . $herecurr);

* Would we like to support any other quotation characters around
  the commit summary?

* I propose to split the error message.
  May it become multi-line?

* How do you think about another wording variant?

  The title must be specified as a single line (without line breaks).

* Would you like to point questionable commit titles out?

Regards,
Markus

  reply	other threads:[~2020-04-28  6:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-28  2:02 Wang YanQing
2020-04-28  6:21 ` Markus Elfring [this message]
2020-04-28 14:06   ` Wang YanQing
2020-04-28 14:54     ` [v3] " Markus Elfring
2020-04-28 10:52 ` [PATCH v3] " Markus Elfring
2020-04-28 16:18   ` Wang YanQing
2020-04-29 15:18     ` [v3] " Markus Elfring
2020-04-28 16:10 ` [PATCH v3] " Joe Perches
2020-04-28 23:17   ` Joe Perches
2020-04-29  6:45     ` [v3] " Markus Elfring

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=35069ae9-cdb4-4728-5e94-6d863dd2721c@web.de \
    --to=markus.elfring@web.de \
    --cc=alexei.starovoitov@gmail.com \
    --cc=apw@canonical.com \
    --cc=joe@perches.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcroce@redhat.com \
    --cc=udknight@gmail.com \
    /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

all inboxes | Powered by JetHome®