Thread (5 messages) 5 messages, 4 authors, 2024-04-01

Re: [PATCH] RFC: add MAINTAINERS file

From: Patrick Steinhardt <hidden>
Date: 2024-03-27 07:17:43

On Sun, Mar 24, 2024 at 07:51:03PM -0700, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
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.
quoted
 - 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.
quoted
 - 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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help