Re: [Ksummit-discuss] [PATCH v2 3/3] libnvdimm, MAINTAINERS: Maintainer Entry Profile

Dan Williams
2019-09-13T12:18:38Z

On Fri, Sep 13, 2019 at 4:49 AM Dan Carpenter <dan.carpenter@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@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.