On Fri, Sep 13, 2019 at 4:49 AM Dan Carpenter <dan.carpenter(a)oracle.com> wrote:
On Fri, Sep 13, 2019 at 01:09:37AM -0600, Jonathan Corbet wrote:
> On Wed, 11 Sep 2019 16:11:29 -0600
> Jens Axboe <axboe(a)kernel.dk> wrote:
> > On 9/11/19 12:43 PM, Dan Carpenter wrote:
> > >
> > > I kind of hate all this extra documentation because now everyone thinks
> > > they can invent new hoops to jump through.
> > FWIW, I completely agree with Dan (Carpenter) here. I absolutely
> > dislike having these kinds of files, and with subsystems imposing weird
> > restrictions on style (like the quoted example, yuck).
> > Additionally, it would seem saner to standardize rules around when
> > code is expected to hit the maintainers hands for kernel releases. Both
> > yours and Martins deals with that, there really shouldn't be the need
> > to have this specified in detail per sub-system.
> This sort of objection came up at the maintainers summit yesterday; the
> consensus was that, while we might not like subsystem-specific rules, they
> do currently exist and we're just documenting reality. To paraphrase
> Phillip K. Dick, reality is that which, when you refuse to document it,
> doesn't go away.
There aren't that many subsystem rules. The big exception is
networking, with the comment style and reverse Chrismas tree
declarations. Also you have to label which git tree the patch applies
to like [net] or [net-next].
It used to be that infiniband used "sizeof foo" instead of sizeof(foo)
but now there is a new maintainer.
There is one subsystem which where the maintainer will capitalize your
patch prefix and complain. There are others where they will silently
change it to lower case. (Maybe that has changed in recent years).
There is one subsystem where the maintainer is super strict rules that
you can't use "I" or "we" in the commit message. So you
can't say "I
noticed a bug while reviewing", you have to say "The code has a bug".
Some maintainers have rules about what you can put in the declaration
block. No kmalloc() in the declarations is a common rule.
"struct foo *p = kmalloc();".
Some people (I do) have strict rules for error handling, but most won't
complain unless the error handling has bugs.
The bpf people want you to put [bpf] or [bpf-next] in the subject.
Everyone just guesses, and uneducated guesses are worse than leaving it
blank, but that's just my opinion.
> So I'm expecting to take this kind of stuff into Documentation/. My own
> personal hope is that it can maybe serve to shame some of these "local
> quirks" out of existence. The evidence from this brief discussion suggests
> that this might indeed happen.
I don't think it's shaming, I think it's validating. Everyone just
insists that since it's written in the Book of Rules then it's our fault
for not reading it. It's like those EULA things where there is more
text than anyone can physically read in a life time.
And the documentation doesn't help. For example, I knew people's rules
about capitalizing the subject but I'd just forget. I say that if you
can't be bothered to add it to checkpatch then it means you don't really
care that strongly.
True, can someone with better perl skills than me take a shot at a
rule for checkpatch to catch the capitalization preference based on
the subsystem being touched, or otherwise agree that if a maintainer
has a changelog capitalization preference they just silently fix it up
at application time and not waste time pointing out something so
trivial? For example, I notice Linus likes "-" instead of "*" for
bullet lists in changelogs he just fixes it up silently if I forget.