Re: [PATCH] RFC: add MAINTAINERS file

[Patrick Steinhardt <ps@pks.im>]

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

On Sun, Mar 24, 2024 at 07:51:03PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > I am more worried about how the file is used and maintained.  Some
> > things to think about while in the "spurred discussion" I can think
> > of are:
> > ...
> >  - Is the project big enough to require this (especially for the
> >    purpose of (1)), or would
> >
> >    $ git shortlog -n --no-merges --since=24.months -- path-to-file
> >
> >    be sufficient and more importantly the value that it will keep
> >    current automatically outweigh the benefit of having this file
> >    that can go stale?  To answer this question, we'd need to know
> >    the turnover rates of past project contributors, of course.  If
> >    it is too high, having such a list may help for (1) and (3)
> >    above.

I don't think of this as "big enough to require this". I rather think
about the onboarding experience for new folks here. Sure, we can ask
them to "Please run git-shortlog(1) to figure out whom to Cc". But if we
instead provide a nice script that does it for them then we make their
lifes easier.

It's also easy to include this for example into GitGitGadget in an
automated way.

> >  - How binding is it for a contributor to be on this list as an area
> >    expert?  Will there be concrete "expected response time"?  It can
> >    be different for each area expert, of course.  I'd expect better
> >    from those who work on Git as a major part of their job and
> >    contributes some part of their work product back to the upstream,
> >    than from folks who do Git as a hobby.  Is each contributer
> >    expected to volunteer to be on this list, with self declared
> >    service level target?

This is a good question. I don't really think that we should enforce any
kind of "service level agreements" here. I think people who are deeply
invested into any of the subsystems are mostly doing a good job of
replying to related patch series already, so I don't see an urgent need
to enforce something here. I would rather assume that we have problems
in areas which _don't_ have an active expert, and I doubt that the
introduction of a "MAINTAINERS" file would help here.

I would thus reformulate the proposal from "MAINTAINERS" to "REVIEWERS".
Instead of saying that person A is a maintainer of subsystem B, it would
say person A has a keen interest in subsystem B and would thus be a very
good candidate to Cc in all your mails touching this subsystem.

> >  - With many good reviewer candidates being employed in companies
> >    and doing Git as part of their job, how would we handle folks
> >    getting transferred out of the Git ecosystem?  Unlike in a
> >    corporate environment, nominating successors who have no track
> >    record in the community by the current area expert would not work
> >    at all.  The successors themselves have to earn respect by
> >    demonstrating their own competence, which would take time.

I think that this problem would go away if we reformulated the problem
to be about discoverability of interested folks instead of setting up
submaintainers.

> So here are some more from the top of my head.
> 
>  - Corollary to "nominating successors from the group at your
>    company may not work well", it may be hard to self-nominate
>    yourself as an area expert if you are not confident that others
>    consider you to be one.

I also think that this becomes less of a problem because you don't have
to be _the_ expert in order to say "I'm curious, please Cc me here".

>  - How authoritative should these "maintainers" be?  Do they have
>    the final say to even override a concensus in a discussion if
>    needed, when clueless discussion participants are drawing a
>    conclusion that would hurt the codebase in the longer term?

Do we actually need this? I'm not too thrilled about people having more
authority simply because they are around longer and have been appointed
as a maintainer. I think that discussions should be decided based on the
merit of arguments, not based on the role one of the participants has.

This is also based on the assumption that experts of a subsystem would
be able to highlight why exactly something is a bad idea and argue
accordingly. Thus, in the ideal case, no authority should be needed
except for the authority that their inherent knowledge already brings
with them.

>  - For whom do we partition the areas?  "For revision walking using
>    connectivity bitmaps, experts are ..." sounds (at least to me)
>    like a plausible and reasonable way to define an expertise area,
>    but the description of the area may be understood only by those
>    who are reasonably familiar with the way how "git log" internally
>    works, for example.  Is it OK to assume that the reader has some
>    basic understanding of how the system works in order to use the
>    maintainer list effectively?
> 
>  - The above worry may be reduced if we partition the area primarily
>    along the file boundaries.  If a set of functions that are not
>    logically related to feature X but has to be in the same
>    compilation unit for some reason live in the file whose primary
>    purpose is to house implementation of the feature X, it may give
>    us an interesting project to figure out how to separate them out
>    and give them "correct" place, and the end result, even though it
>    is a side effect, would be a more modular and readable code.

I think partitioning intersted folks along file boundaries would be the
easiest. Also because...

>  - If we adopt the file format from the kernel project, can we
>    leverage their tooling as well to query the maintainers file?  I
>    thought they have a tool that reads your patch into and figures
>    out what area is being touched to spit out a good set of Cc
>    candidates?

... the tooling from the kernel project already works in this way. They
do have "scripts/get_maintainer.pl" that can be invoked with a set of
files that have changed, and it will then spit out a list of folks to Cc
as declared in the MAINTAINERS file.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]