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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.22.394.2107191324330.14021@anisinha-lenovo>
Date:   Mon, 19 Jul 2021 13:38:26 +0530 (IST)
From:   Ani Sinha <ani@...sinha.ca>
To:     Joe Perches <joe@...ches.com>
cc:     Ani Sinha <ani@...sinha.ca>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        anirban.sinha@...ia.com, mikelley@...rosoft.com,
        Andy Whitcroft <apw@...onical.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>
Subject: Re: [PATCH v3] checkpatch: add a rule to check general block comment
 style

On Mon, 19 Jul 2021, Joe Perches wrote:

> On Mon, 2021-07-19 at 12:25 +0530, Ani Sinha wrote:
>
> > I do not see why we cannot add this rule to checkpatch. If the
> > reviewer likes the other style of commenting they can always ask for a
> > correction. Having checkpatch agree with Linus' preferred style of
> > commenting and the preferred documeted style of commenting (which seems to
> > be the same) does make everything uniform and agreeable.
>
> Too many novice developers take checkpatch output as dicta.
>
> It's not.
>

Well those "novice" developers have perhaps worked in companies where
tooling like pre-commit sanity hooks have provided immense value in
ensuring certain basic rules and code quality is maintained across the
board, particulay when the company scales. Existing violations did not
deter them from adding stricter rules to make sure all future commits
follow certain patterns. Ofcourse at the end of the day, common sense
trumps any tooling, goes without saying.

> It's just produces suggestions that should _always_ be taken
> not very seriously.  Those suggestions should perhaps be
> considered, but good taste should always override a brainless
> script.

At the very very least, checkpatch should lay this out in clear terms
every time this is run. Different communities have different rules and for
me, I always run all my patches through checkpatch to make sure that the
patch I sent out of review at least is checkpatch clean. This makes sure
that I am not violating any obvious code submission rules laid out by that
community. This is particularly true for kernel community where flaming
people for even small issues seems to be the culture!

>
> _Very_ few senior developers really care that much about any
> particular comment style.

I disagree on this.

>
> These are the same senior developers that would be burdened
> with unnecessary patches to review from those novice developers
> that believe checkpatch should always be followed.
>

Well for those "novice" developers, the doc tells us to run checkpatch
and address the complaints :

 Are you sure your patch is free of silly mistakes?  You should always
   run patches through scripts/checkpatch.pl and address the complaints it
   comes up with.

Anyways it seems this conversation is self serving for the kernel's sr
developers so that they can take any stance convenient to them.
There is no value.