Re: [PATCH v3] checkpatch: add a rule to check general block comment style

[Ani Sinha <ani@anisinha.ca>]

[-- Attachment #1: Type: text/plain, Size: 3695 bytes --]



On Sun, 18 Jul 2021, Joe Perches wrote:

> On Sun, 2021-07-18 at 19:08 +0530, Ani Sinha wrote:
> > On Sun, 18 Jul 2021, Lukas Bulwahn wrote:
> > > On Fri, Jul 16, 2021 at 6:15 PM Ani Sinha <ani@anisinha.ca> wrote:
> > > > checkpatch maintainers, any comments?
> > > > On Wed, 14 Jul 2021, Ani Sinha wrote:
> > > > > The preferred style for long (multi-line) comments is:
> > > > >
> > > > > .. code-block:: c
> > > > >
> > > > >       /*
> > > > >        * This is the preferred style for multi-line
> > > > >        * comments in the Linux kernel source code.
> > > > >        * Please use it consistently.
> > > > >        *
> > > > >        * Description:  A column of asterisks on the left side,
> > > > >        * with beginning and ending almost-blank lines.
> > > > >        */
> > > > >
> > > > > It seems rule in checkpatch.pl is missing to ensure this for
> > > > > non-networking related changes. This patch adds this rule.
> []
> > > Honest feedback: IMHO, your commit message is unreadable and incomprehensible.
> >
> > OK. However, I fail to see how your above comment is useful without any
> > suggestion as to how to improve the commit log. I find having some test
> > data with the commit message valuable so that there is some sort of record
> > as to how the change was tested and with what arguments. Beyond that this
> > is not something I am really worried about. The commit message can be
> > modified and improved in any way reviewers like.
> >
> > >
> > > Now to the feature you are proposing:
> > >
> > > I do not think that it is good if checkpatch would point out a quite
> > > trivial syntactic issue that probably is currently violated many times
> > > (>10,000 or even >100,000 times?) in the overall repository. That will
> > > make checkpatch warn on many commits with this check and divert the
> > > attention from other checks that are more important than the style of
> > > starting comments.
> >
> > I have some strong opinions on this. Just because a rule has been violated
> > in the past does not mean it can continue to be violated in the future.
>
> Intensity of opinion varies considerably here.
>
> > > Further, some evaluation by Aditya shows that the distinction between
> > > NETWORKING COMMENT STYLE and GENERAL KERNEL COMMENT STYLE is not as
> > > easily split as currently encoded in the checkpatch script,
> > > https://lore.kernel.org/linux-kernel-mentees/cfff5784-9ca3-07f8-c51c-f1c82b2871e3@gmail.com/.
> > > So, this checkpatch check is largely wrong already as of now and most
> > > probably ignored by many contributors.
>
> The only reason the rule exists at all is because the networking maintainer
> was constantly telling people to change the comment style in patches.
>
> I don't care one way or another.
>
> // comments are fine
> /* comments are fine */
>
> In networking, multiline comments are almost exclusively like
> what Linus himself does not like:
>
> 	/* comment
> 	 * ...
> 	 */
>
> but in other subsystems, the styles of multiline comments varies.
>
> Either works, there is no single standard.
>

OK then in that case, maybe update
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v5.14-rc2#n584

It is confusing to patch submitters (and it happened to me with a recent
patch) that the reviewer insists on a particular commenting style when
checkpatch does not enforce it. Its confusing for reviewers too.


> And as the referenced link by Aditya somewhat shows, the nominal
> rule compliance varies by the age of the code.  No one care much
> about code submitted a couple decades ago for subsystems and drivers
> that are effectively obsolete...
>
>
>