| Message ID | pull.1694.git.git.1711164460562.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | RFC: add MAINTAINERS file | expand |
"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Linus Arver <linusa@google.com> > > This patch is designed to spur discussion about adding an official > MAINTAINERS file to our project. The hope is that it could be used as a > reference in (at least) the following scenarios: > > (1) [CC list] patch authors want to know who to CC on their > submissions, without resorting to git-blame-level of precision; > > (2) [escalation path] patch authors have been waiting 1+ weeks for > review comments, but are not sure who to escalate to (other than > Junio); > > (3) [status tracking] record former maintainers/reviewers who are now > inactive. > > In addition having a MAINTAINERS file could give a more official sense > of ownership in the codebase. OK. They are understandable goals. As to the format of the actual file, I do not have much opinion. What works for the kernel may or may not work for us, as the project size is very different, but I am fairly confident that we can agree on something usable. 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. - 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? - 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. There may be many others. Thanks. > The MAINTAINERS file here is stolen from the one used in the Linux > Kernel. We do not have to follow its format at all; it is merely added > here as a reference for comparison and prior art. > > Signed-off-by: Linus Arver <linusa@google.com> > --- > RFC: add MAINTAINERS file > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1694%2Flistx%2Fmaintainers-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1694/listx/maintainers-v1 > Pull-Request: https://github.com/git/git/pull/1694 > > MAINTAINERS | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 85 insertions(+) > create mode 100644 MAINTAINERS > > diff --git a/MAINTAINERS b/MAINTAINERS > new file mode 100644 > index 00000000000..34fa3baf3a5 > --- /dev/null > +++ b/MAINTAINERS > @@ -0,0 +1,85 @@ > +List of maintainers > +=================== > + > +Descriptions of section entries and preferred order > +--------------------------------------------------- > + > + M: *Mail* patches to: FullName <address@domain> > + R: Designated *Reviewer*: FullName <address@domain> > + These reviewers should be CCed on patches. > + L: *Mailing list* that is relevant to this area > + S: *Status*, one of the following: > + Supported: Someone is actually paid to look after this. > + Maintained: Someone actually looks after it. > + Odd Fixes: It has a maintainer but they don't have time to do > + much other than throw the odd patch in. See below.. > + Orphan: No current maintainer [but maybe you could take the > + role as you write your new code]. > + Obsolete: Old code. Something tagged obsolete generally means > + it has been replaced by a better system and you > + should be using that. > + W: *Web-page* with status/info > + Q: *Patchwork* web based patch tracking system site > + B: URI for where to file *bugs*. A web-page with detailed bug > + filing info, a direct bug tracker link, or a mailto: URI. > + C: URI for *chat* protocol, server and channel where developers > + usually hang out, for example irc://server/channel. > + P: *Subsystem Profile* document for more details submitting > + patches to the given subsystem. This is either an in-tree file, > + or a URI. See Documentation/maintainer/maintainer-entry-profile.rst > + for details. > + T: *SCM* tree type and location. > + Type is one of: git, hg, quilt, stgit, topgit > + F: *Files* and directories wildcard patterns. > + A trailing slash includes all files and subdirectory files. > + F: drivers/net/ all files in and below drivers/net > + F: drivers/net/* all files in drivers/net, but not below > + F: */net/* all files in "any top level directory"/net > + One pattern per line. Multiple F: lines acceptable. > + X: *Excluded* files and directories that are NOT maintained, same > + rules as F:. Files exclusions are tested before file matches. > + Can be useful for excluding a specific subdirectory, for instance: > + F: net/ > + X: net/ipv6/ > + matches all files in and below net excluding net/ipv6/ > + N: Files and directories *Regex* patterns. > + N: [^a-z]tegra all files whose path contains tegra > + (not including files like integrator) > + One pattern per line. Multiple N: lines acceptable. > + scripts/get_maintainer.pl has different behavior for files that > + match F: pattern and matches of N: patterns. By default, > + get_maintainer will not look at git log history when an F: pattern > + match occurs. When an N: match occurs, git log history is used > + to also notify the people that have git commit signatures. > + K: *Content regex* (perl extended) pattern match in a patch or file. > + For instance: > + K: of_get_profile > + matches patches or files that contain "of_get_profile" > + K: \b(printk|pr_(info|err))\b > + matches patches or files that contain one or more of the words > + printk, pr_info or pr_err > + One regex pattern per line. Multiple K: lines acceptable. > + > +Maintainers List > +---------------- > + > +.. note:: When reading this list, please look for the most precise areas > + first. When adding to this list, please keep the entries in > + alphabetical order. > + > +3C59X NETWORK DRIVER > +M: Steffen Klassert <klassert@kernel.org> > +L: netdev@vger.kernel.org > +S: Odd Fixes > +F: Documentation/networking/device_drivers/ethernet/3com/vortex.rst > +F: drivers/net/ethernet/3com/3c59x.c > + > +... > + > +THE REST > +M: Linus Torvalds <torvalds@linux-foundation.org> > +L: linux-kernel@vger.kernel.org > +S: Buried alive in reporters > +T: git git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > +F: * > +F: */ > > base-commit: 11c821f2f2a31e70fb5cc449f9a29401c333aad2
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. > > - 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? > > - 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. > > There may be many others. 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. - 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? - 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. - 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? - Can contrib/contacts/git-contacts be taught about this new source of information, and if so how? - Once we start breaking down the system into expertise areas, are there areas without any existng experts already? If you send patches to the list right now in the following areas, I do not think you'll find capable reviewers whose acks weigh well enough [*]: gitk, git-gui, contrib/completion, git-p4, gitweb, git-svn. * Please raise a hand and say "No, you know I am very familiar with that area; you just simply forgot about me because we have not seen any patches in the area recently". - When there are no active area experts, what would the default action be? We would risk degrading the quality of such "neglected" part of the system if we adopt "anything gets accepted blindly" approach, so I would really want to avoid it. - When an area with incumbent experts sees interest from some developers, it is the best for these new people to demonstrate their own competence and earn community's trust to eventually become the area experts themselves, but that may not be so easy in practice due to chicken-and-egg problem.
Junio C Hamano <gitster@pobox.com> writes: > "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Linus Arver <linusa@google.com> >> >> This patch is designed to spur discussion about adding an official >> MAINTAINERS file to our project. The hope is that it could be used as a >> reference in (at least) the following scenarios: >> >> (1) [CC list] patch authors want to know who to CC on their >> submissions, without resorting to git-blame-level of precision; >> >> (2) [escalation path] patch authors have been waiting 1+ weeks for >> review comments, but are not sure who to escalate to (other than >> Junio); >> >> (3) [status tracking] record former maintainers/reviewers who are now >> inactive. >> >> In addition having a MAINTAINERS file could give a more official sense >> of ownership in the codebase. > > OK. They are understandable goals. > > As to the format of the actual file, I do not have much opinion. > What works for the kernel may or may not work for us, as the project > size is very different, but I am fairly confident that we can agree > on something usable. > > 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. > > - 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? > > - 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. > > There may be many others. Thanks for the initial comments! I will try to formulate a response soon while I consider your other comments also.
On Sat, Mar 23, 2024 at 12:19:15PM -0700, Junio C Hamano wrote: > - 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 might be biased, but I think that we are not quite there, yet. Subjectively, I find myself working in areas where I mostly know who to CC based on what parts of the tree that I'm touching. But in the cases where I do not, the shortlog --since=2.years.ago is usually pretty small. The output below lists number of individuals in the right-hand column, and the number of files with that many individuals having touched it in the last two years in the left-hand column: for f in $(git ls-files **/*.{c,h}) do git shortlog -s --since-as-filter=2.years.ago -- $f | wc -l \ || return 1 done | sort -n | uniq -c | sort -rnk1 192 1 160 0 112 2 94 3 80 4 68 5 40 6 30 9 27 8 25 7 19 11 12 10 11 12 5 17 5 14 3 13 2 18 2 16 1 22 1 20 1 19 1 15 So a vast majority of *.ch files have fewer than 10 individuals working on it in the past two years. By my count, there are 891 total source files matching **/*.{c,h}, 828 of which have fewer than 10 people working on it. IOW, ~92.3% of the project is touched by no more than 9 people in the last two years. That kind of scale doesn't strike me as something that needs something like a MAINTAINERS file to help make sense of it. It's possible that some of the files that see more contributors might need some sort of aide, but there are so few of them I have a hard time imagining it. > - 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? I share your concern here, too. Another thought that comes to mind is the difference between "maintainer" and "reviewer". For a file with, say, 4 committers in the past two years, I imagine that as the maintainer that you'd give about equal weight to any of their reviews (with obvious exceptions, like if someone had showed up in the shortlog over a much longer period, or had significantly more or substantial patches in a given area). Those kinds of things are hard to quantify exactly, and perhaps that is the point of a MAINTAINERS file. But I think quantifying those things matters a lot more when you have dozens or more individuals contributing to files across the tree, and the numbers above show that (at least for a large majority of the project) we're simply not there yet. Thanks, Taylor
Taylor Blau <me@ttaylorr.com> writes: >> - 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? > > I share your concern here, too. But I wasn't expressing any concern above ;-) I'd consider it a progress if we can give contributors (and the maintainer, too) more predictable review experience. If we can even optionally give some assurance on the response time, e.g., "I'll to respond to and usher to completion any patches in this area if they are promising within X days; I may not respond to all patches and certainly not to ones that I do not find interesting" would already be better than some patches that do not see any reviews for three weeks without such an "optional" maintainer. > Those kinds of things are hard to quantify exactly, and perhaps that is > the point of a MAINTAINERS file. Yeah, I am not interested in what exact form such a list of folks who are willing to help guiding topics along comes from. What I am hoping to find out is if we can come up with a bit more structured way to say "yes" or "no" to topics, rather than the current "nobody may be interested in a topic, in which case it is anybody's guess what will happen to it" (actually the default is "to drop", and I often end up to be "somebody who gets sympathetic and reads the topic to salvage, instead of the default action that is to drop"). Thanks.
Junio C Hamano <gitster@pobox.com> writes: > "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Linus Arver <linusa@google.com> >> >> This patch is designed to spur discussion about adding an official >> MAINTAINERS file to our project. The hope is that it could be used as a >> reference in (at least) the following scenarios: >> >> (1) [CC list] patch authors want to know who to CC on their >> submissions, without resorting to git-blame-level of precision; >> >> (2) [escalation path] patch authors have been waiting 1+ weeks for >> review comments, but are not sure who to escalate to (other than >> Junio); >> >> (3) [status tracking] record former maintainers/reviewers who are now >> inactive. >> >> In addition having a MAINTAINERS file could give a more official sense >> of ownership in the codebase. > > OK. They are understandable goals. > > As to the format of the actual file, I do not have much opinion. > What works for the kernel may or may not work for us, as the project > size is very different, but I am fairly confident that we can agree > on something usable. Agreed. > 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. In addition to checking git-shortlog on the Git repo, perhaps it's also worth running a similar query against the public-inbox repo of this list? We could perhaps use a script to generate this list automatically every Git release (or some other cadence that we undergo regularly)? > - 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? Ideally there should be some teeth to the document/agreement (esp for service level targets), but I think practically the best we can do is positive reinforcement. So maybe a prominent "The Git Code Review Team" web page (somewhere on git-scm.com?) with profile photos and short biographies should be enough to motivate people to stay engaged and keep their spot. I realize that such an idea is beyond the scope of a simple MAINTAINERS (or similar) file that's checked into the Git code repo, but I think it's worth stating as a thought experiment. The overall point I want to make is that we need to be extra-thankful to those who sign up to say "yes, I can review patches in areas X, Y, Z" and recognize (in a very official way) their generosity in contributing back to this project. > - 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. Unfortunately I don't think there's a good answer here. I agree that only those who have demonstrated a good track record should become a "successor". OTOH, if we are fortunate enough to have multiple people sign up for a particular area, then maybe that can be a sub-team and finding a successor won't be such a big deal. It would only be a problem for those areas where there is only 1 person who signed up for it.
Oops, I should have just responded to this email once as you quoted your original reply. Sorry for the inconvenience. BTW thanks for spurring on the discussion. I've also CC'ed additional folks directly to try to get more feedback for this topic. Junio C Hamano <gitster@pobox.com> writes: > Junio C Hamano <gitster@pobox.com> writes: > > [...] > > 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. There probably needs to be some sort of voting mechanism to affirm membership into the "maintainers" group. > - 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? I want to say "it depends", but for sake of simplicity, I think "yes" is a better answer than "no". If the existing area expert is not happy with the direction of a patch series... tough luck, try again. If we end up overruling the opinions of established experts very often then the title of "maintainer" loses teeth and I don't like that consequence. IIUC in the Linux Kernel people pull large chunks of changes from each other until it all converges into the one owned by Torvalds. That model works because of the "network of trust" amongst the various maintainers, each maintainer being an area expert. I'm digressing a little bit here, but perhaps the "area expert" model will only work if we move to a more Kernel-like model with multiple separate "collection points" of patches (where there are multiple maintainers). But I don't think Git is large enough where that model makes sense. I'm not sure. > - 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? I think that assumption is fine. We have to start somewhere. BTW I expect such a "maintainers" doc to evolve a bit as we hit bumps on the road along the way. > - 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. Yup, file boundaries may be the simplest one. Another partitioning dimension might be individual internal libraries (header files to start, although some things like git-compat-util.h might need more than one expert due to its complexity). > - 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? Yes. I found guidance [1] which suggests using scripts/get_maintainer.pl [2] to find the CC list. And this script appears [3] to use the MAINTAINERS file format to do its work. I'm not a big fan of Perl but I suppose as long as we keep the same format we can always just reuse that script (assuming it doesn't have Kernel-only things hardcoded into it that make it unusable outside the Kernel). > - Can contrib/contacts/git-contacts be taught about this new source > of information, and if so how? Wow, I had no idea this existed. One (stupid) idea would be to do a set union operation with the output of git-contacts with the output of scripts/get_maintainer.pl. > - Once we start breaking down the system into expertise areas, are > there areas without any existng experts already? If you send > patches to the list right now in the following areas, I do not > think you'll find capable reviewers whose acks weigh well enough > [*]: gitk, git-gui, contrib/completion, git-p4, gitweb, git-svn. > > * Please raise a hand and say "No, you know I am very familiar > with that area; you just simply forgot about me because we > have not seen any patches in the area recently". I think the lack of experts in certain areas is fine. It may even be thee case that there are only a few experts (in a few areas) around to start with. > - When there are no active area experts, what would the default > action be? We would risk degrading the quality of such > "neglected" part of the system if we adopt "anything gets > accepted blindly" approach, so I would really want to avoid it. Perhaps we could have a team of "code reviewers" who can be available to review patches at some basic level, not at the level of experts? Over time such reviewers could graduate to become an area expert they like to work on. The idea is to make sure that there's always a steady pipeline of would-be-experts who are ready to join the "maintainers" ranks, when the existing maintainers inevitably become inactive or move on from the project. Anyway, to answer your immediate question, I think the default action would still be to wait for code reviews from known people (who are not new to the community). > - When an area with incumbent experts sees interest from some > developers, it is the best for these new people to demonstrate > their own competence and earn community's trust to eventually > become the area experts themselves, but that may not be so easy > in practice due to chicken-and-egg problem. Yeah I agree. I guess I anticipated this problem with my response just above WRT having a pipeline of would-be-experts. To restate, I think it's important to have a healthy number of people across all levels of expertise (beginner, intermediate, advanced) in the community. Maybe we have that already? If so, the trick would be to incentivize the beginners and intermediate level folks to stick around long enough to become experts. The GSoC [4] program is one way, although I don't know how successful it has been (and I don't think it's very scalable). [1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#select-the-recipients-for-your-patch [2] https://github.com/torvalds/linux/blob/master/scripts/get_maintainer.pl [3] https://github.com/torvalds/linux/blob/7033999ecd7b8cf9ea59265035a0150961e023ee/scripts/get_maintainer.pl#L345 [4] https://summerofcode.withgoogle.com/
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
Linus Arver <linusa@google.com> writes: > I realize that such an idea is beyond the scope of a simple MAINTAINERS > (or similar) file that's checked into the Git code repo, but I think > it's worth stating as a thought experiment. As we already have agreed that neither of us care the exact format of the file (yet), regardless of how a contributor, who is about to send a patch, will find an area "maintainer" to help the patch along the process, it is far more important to discuss and decide what responsibilities and authorities are expected of these maintainers. The development community has been fairly loosely organized so far, but I'd like to see responsibility and authority spread a bit more widely yet still not too thinly to compromise the project integrity. > The overall point I want to make is that we need to be > extra-thankful to those who sign up to say "yes, I can review > patches in areas X, Y, Z" and recognize (in a very official way) > their generosity in contributing back to this project. Yup.
Junio C Hamano <gitster@pobox.com> writes: > Linus Arver <linusa@google.com> writes: > >> I realize that such an idea is beyond the scope of a simple MAINTAINERS >> (or similar) file that's checked into the Git code repo, but I think >> it's worth stating as a thought experiment. > > As we already have agreed that neither of us care the exact format > of the file (yet), regardless of how a contributor, who is about to > send a patch, will find an area "maintainer" to help the patch along > the process, it is far more important to discuss and decide what > responsibilities and authorities are expected of these maintainers. I'm starting to think that the new responsibility should be as small as possible, and build from there. So the smallest bit of (initial?) responsibility expected of the new roster of maintainers could be "maintainer must respond to CC pings on the list within 7 days". For those who have more time to spend on the project, the next rung of responsibility could be "maintainer is available to review patches outside of their domain of expertise if no one else has reviewed the series in 7 days". I haven't thought too much about the "authority" part yet. > The development community has been fairly loosely organized so far, > but I'd like to see responsibility and authority spread a bit more > widely yet still not too thinly to compromise the project integrity. Agreed.
Patrick Steinhardt <ps@pks.im> writes: > On Sun, Mar 24, 2024 at 07:51:03PM -0700, Junio C Hamano wrote: >> Junio C Hamano <gitster@pobox.com> writes: > [...] > > 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. Good idea!
Linus Arver <linusa@google.com> writes: > Patrick Steinhardt <ps@pks.im> writes: > >> On Sun, Mar 24, 2024 at 07:51:03PM -0700, Junio C Hamano wrote: >>> Junio C Hamano <gitster@pobox.com> writes: >> [...] >> >> 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. > > Good idea! Seconded.
On Wed, Mar 27, 2024 at 08:17:38AM +0100, Patrick Steinhardt wrote: > 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. Do you think that the script in contrib/contacts does a sufficient job at this? I admit that I am not a frequent user of it (mostly because I end up either having a good sense of who I want to review patches ahead of time, and/or I end up just running 'shortlog'), so I can't vouch for its accuracy. But from running it on a handful of patches just now locally while replying to your email, it seems to do a reasonable job at identifying a good set of candidate reviewers. Perhaps we haven't been as good at advertising this script as we could be, and that's why it isn't as widely used as it could be? I'm not sure. Thanks, Taylor
Taylor Blau <me@ttaylorr.com> writes: >> 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. > > Do you think that the script in contrib/contacts does a sufficient job > at this? Yup, that was my first reaction. If it is not sufficient to mechanically mine history with shortlog or blame or contacts, and if we can add a high quality hand-curated input to improve the result contacts gives us, that would be a progress. I view the MAINTAINERS format just one way to give such human generated input. > Perhaps we haven't been as good at advertising this script as we could > be, and that's why it isn't as widely used as it could be? I'm not sure. Good point. Do we even mention it in MyFirstSomething docs?
Junio C Hamano <gitster@pobox.com> writes: > Taylor Blau <me@ttaylorr.com> writes: > > [...] >> Perhaps we haven't been as good at advertising this script as we could >> be, and that's why it isn't as widely used as it could be? I'm not sure. > > Good point. Do we even mention it in MyFirstSomething docs? I've pushed up a patch for review to mention it in our docs: https://lore.kernel.org/git/pull.1704.git.1712017205754.gitgitgadget@gmail.com (And, sorry for not simply pushing the patch to this thread because I only know how to use GGG currently... Cheers)
On Mon, Apr 01, 2024 at 03:13:27PM -0700, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > > >> 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. > > > > Do you think that the script in contrib/contacts does a sufficient job > > at this? I admittedly never used it much, either. I didn't see a lot of benefit because I can figure out myself whom to Cc. That would be less true for newcomers to the community though, where it may be more useful. > Yup, that was my first reaction. If it is not sufficient to > mechanically mine history with shortlog or blame or contacts, and if > we can add a high quality hand-curated input to improve the result > contacts gives us, that would be a progress. I view the MAINTAINERS > format just one way to give such human generated input. Agreed. I don't think that we have to pick either MAINTAINERS or the "contrib/contacts" script, but would rather want the existing script to honor MAINTAINERS as an additional data source. When it does know about both I also see myself using it more frequently in the future. It would be nice if git-send-email(1)/git-format-patch(1) had a switch `--cc-command=` or similar that you can pass the script to so that To/Cc lines would be added automatically. The script then gets the commit range as input and can decide based on whatever criteria whom to Cc. To the best of my knowledge that is not currently possible. Patrick
On Tue, Apr 2, 2024 at 1:40 AM Patrick Steinhardt <ps@pks.im> wrote: > When it does know about both I also see myself using it more frequently > in the future. It would be nice if git-send-email(1)/git-format-patch(1) > had a switch `--cc-command=` or similar that you can pass the script to > so that To/Cc lines would be added automatically. The script then gets > the commit range as input and can decide based on whatever criteria whom > to Cc. To the best of my knowledge that is not currently possible. I may be misunderstanding your statement, but this automated mode was exactly the original use-case. contrib/contacts/git-contacts.txt says this: This command can be useful for determining the list of people with whom to discuss proposed changes, or for finding the list of recipients to Cc: when submitting a patch series via `git send-email`. For the latter case, `git contacts` can be used as the argument to `git send-email`'s `--cc-cmd` option.
On Tue, Apr 02, 2024 at 01:46:22AM -0400, Eric Sunshine wrote: > On Tue, Apr 2, 2024 at 1:40 AM Patrick Steinhardt <ps@pks.im> wrote: > > When it does know about both I also see myself using it more frequently > > in the future. It would be nice if git-send-email(1)/git-format-patch(1) > > had a switch `--cc-command=` or similar that you can pass the script to > > so that To/Cc lines would be added automatically. The script then gets > > the commit range as input and can decide based on whatever criteria whom > > to Cc. To the best of my knowledge that is not currently possible. > > I may be misunderstanding your statement, but this automated mode was > exactly the original use-case. contrib/contacts/git-contacts.txt says > this: > > This command can be useful for determining the list of people with > whom to discuss proposed changes, or for finding the list of > recipients to Cc: when submitting a patch series via `git > send-email`. For the latter case, `git contacts` can be used as > the argument to `git send-email`'s `--cc-cmd` option. Ah. I myself use git-format-patch(1) and mutt(1) to send the resulting patches, and that command doesn't know about `--cc-cmd`. But git-send-email(1) in fact does, good to know. So my statement still partially stands, and we might want to make `--cc-cmd` available in git-format-patch(1), too. Patrick
On Sat, Mar 30, 2024 at 10:59:53AM -0700, Linus Arver wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > Linus Arver <linusa@google.com> writes: > > > >> I realize that such an idea is beyond the scope of a simple MAINTAINERS > >> (or similar) file that's checked into the Git code repo, but I think > >> it's worth stating as a thought experiment. > > > > As we already have agreed that neither of us care the exact format > > of the file (yet), regardless of how a contributor, who is about to > > send a patch, will find an area "maintainer" to help the patch along > > the process, it is far more important to discuss and decide what > > responsibilities and authorities are expected of these maintainers. > > I'm starting to think that the new responsibility should be as small as > possible, and build from there. So the smallest bit of (initial?) > responsibility expected of the new roster of maintainers could be > "maintainer must respond to CC pings on the list within 7 days". > > For those who have more time to spend on the project, the next rung of > responsibility could be "maintainer is available to review patches > outside of their domain of expertise if no one else has reviewed the > series in 7 days". > > I haven't thought too much about the "authority" part yet. One thing that makes me feel a bit uneasy about the authority part is that contributors to Git are quite often direct competitors on the company level, as well. This never has been a problem in the past, quite on the contrary: I really value the cross-competitor collaboration we have in this project. But I have to wonder what it can potentially lead to if we did assign more authority to some contributors. Theoretically speaking, that would allow for sabotaging interests of a direct competitor. Mind you, I don't think this would happen in the current state of the project. I'm merely trying to think about worst-case scenarios, which may or may not be helpful in this context. Patrick > > The development community has been fairly loosely organized so far, > > but I'd like to see responsibility and authority spread a bit more > > widely yet still not too thinly to compromise the project integrity. > > Agreed.
On Wed, Mar 27, 2024 at 06:29:13AM -0700, Junio C Hamano wrote: > The development community has been fairly loosely organized so far, > but I'd like to see responsibility and authority spread a bit more > widely yet still not too thinly to compromise the project integrity. I guess the main motivation of this statement is to reduce your load in particular, right? If so, do you have any particular pain points that can be spread across the community that would help you? Or is it really only spreading the review load by relying more on subsystem-maintainers? Patrick
Patrick Steinhardt <ps@pks.im> writes: > On Wed, Mar 27, 2024 at 06:29:13AM -0700, Junio C Hamano wrote: >> The development community has been fairly loosely organized so far, >> but I'd like to see responsibility and authority spread a bit more >> widely yet still not too thinly to compromise the project integrity. > > I guess the main motivation of this statement is to reduce your load in > particular, right? If so, do you have any particular pain points that > can be spread across the community that would help you? Or is it really > only spreading the review load by relying more on subsystem-maintainers? It is not for load reduction, per-se. I wish people spent more time and effort on reviewing and helping others to polish topics as much as writing their own topic. We see corporate sponsored entities propose a feature, polish it with reviewers, and then after the topic lands and collect their perf scores, leave it bitrot, making it "the community's problem" to maintain it. They may even have to leave the project due to reorg. Somebody comes with a patch in such an area, and receives no response. The only two ways I see to reduce such "abandoned parts" of the system are: * Those who are still with the project but their interest moved to other areas in the project to come back to help in the area they were involved in, and * Those who haven't been involved in an area to grow expertise in it. The latter is far more sustainable than the former. People forget and become no longer more expert than others who have fresh interst in the same area. But for that to happen, we first need to know where the gaps of expertise coverage are. If we try to spread responsibility and authority, we will quickly identify these areas as we find no experts in these areas.
Patrick Steinhardt <ps@pks.im> writes: > On Sat, Mar 30, 2024 at 10:59:53AM -0700, Linus Arver wrote: >> Junio C Hamano <gitster@pobox.com> writes: >> >> > Linus Arver <linusa@google.com> writes: >> > >> >> I realize that such an idea is beyond the scope of a simple MAINTAINERS >> >> (or similar) file that's checked into the Git code repo, but I think >> >> it's worth stating as a thought experiment. >> > >> > As we already have agreed that neither of us care the exact format >> > of the file (yet), regardless of how a contributor, who is about to >> > send a patch, will find an area "maintainer" to help the patch along >> > the process, it is far more important to discuss and decide what >> > responsibilities and authorities are expected of these maintainers. >> >> I'm starting to think that the new responsibility should be as small as >> possible, and build from there. So the smallest bit of (initial?) >> responsibility expected of the new roster of maintainers could be >> "maintainer must respond to CC pings on the list within 7 days". >> >> For those who have more time to spend on the project, the next rung of >> responsibility could be "maintainer is available to review patches >> outside of their domain of expertise if no one else has reviewed the >> series in 7 days". >> >> I haven't thought too much about the "authority" part yet. > > One thing that makes me feel a bit uneasy about the authority part is > that contributors to Git are quite often direct competitors on the > company level, as well. This never has been a problem in the past, quite > on the contrary: I really value the cross-competitor collaboration we > have in this project. > > But I have to wonder what it can potentially lead to if we did assign > more authority to some contributors. Theoretically speaking, that would > allow for sabotaging interests of a direct competitor. > > Mind you, I don't think this would happen in the current state of the > project. I'm merely trying to think about worst-case scenarios, which > may or may not be helpful in this context. No problem (I also like to think worst-case scenarios, so thanks for the thought experiment). Initially I agreed with the concerns you raised, but on further thinking I don't have the same concerns any more, for two reasons. (1) It's impossible to tell if someone is actually intentionally sabotaging the interests of a competitor --- simply because no one will admit to doing so openly on this list. (2) Even if we do have authority figures on this project, if they block a patch series from being merged, the reasons they give must remain purely technical. Otherwise, I think such authority figures will compromise (lose) their reputation pretty quickly. For (2) it could be that they could block something for both $DAYJOB and technical reasons, but I think this is still fine. The fact that they have $DAYJOB reasons wouldn't take away any merit from the technical reasons.
diff --git a/MAINTAINERS b/MAINTAINERS new file mode 100644 index 00000000000..34fa3baf3a5 --- /dev/null +++ b/MAINTAINERS @@ -0,0 +1,85 @@ +List of maintainers +=================== + +Descriptions of section entries and preferred order +--------------------------------------------------- + + M: *Mail* patches to: FullName <address@domain> + R: Designated *Reviewer*: FullName <address@domain> + These reviewers should be CCed on patches. + L: *Mailing list* that is relevant to this area + S: *Status*, one of the following: + Supported: Someone is actually paid to look after this. + Maintained: Someone actually looks after it. + Odd Fixes: It has a maintainer but they don't have time to do + much other than throw the odd patch in. See below.. + Orphan: No current maintainer [but maybe you could take the + role as you write your new code]. + Obsolete: Old code. Something tagged obsolete generally means + it has been replaced by a better system and you + should be using that. + W: *Web-page* with status/info + Q: *Patchwork* web based patch tracking system site + B: URI for where to file *bugs*. A web-page with detailed bug + filing info, a direct bug tracker link, or a mailto: URI. + C: URI for *chat* protocol, server and channel where developers + usually hang out, for example irc://server/channel. + P: *Subsystem Profile* document for more details submitting + patches to the given subsystem. This is either an in-tree file, + or a URI. See Documentation/maintainer/maintainer-entry-profile.rst + for details. + T: *SCM* tree type and location. + Type is one of: git, hg, quilt, stgit, topgit + F: *Files* and directories wildcard patterns. + A trailing slash includes all files and subdirectory files. + F: drivers/net/ all files in and below drivers/net + F: drivers/net/* all files in drivers/net, but not below + F: */net/* all files in "any top level directory"/net + One pattern per line. Multiple F: lines acceptable. + X: *Excluded* files and directories that are NOT maintained, same + rules as F:. Files exclusions are tested before file matches. + Can be useful for excluding a specific subdirectory, for instance: + F: net/ + X: net/ipv6/ + matches all files in and below net excluding net/ipv6/ + N: Files and directories *Regex* patterns. + N: [^a-z]tegra all files whose path contains tegra + (not including files like integrator) + One pattern per line. Multiple N: lines acceptable. + scripts/get_maintainer.pl has different behavior for files that + match F: pattern and matches of N: patterns. By default, + get_maintainer will not look at git log history when an F: pattern + match occurs. When an N: match occurs, git log history is used + to also notify the people that have git commit signatures. + K: *Content regex* (perl extended) pattern match in a patch or file. + For instance: + K: of_get_profile + matches patches or files that contain "of_get_profile" + K: \b(printk|pr_(info|err))\b + matches patches or files that contain one or more of the words + printk, pr_info or pr_err + One regex pattern per line. Multiple K: lines acceptable. + +Maintainers List +---------------- + +.. note:: When reading this list, please look for the most precise areas + first. When adding to this list, please keep the entries in + alphabetical order. + +3C59X NETWORK DRIVER +M: Steffen Klassert <klassert@kernel.org> +L: netdev@vger.kernel.org +S: Odd Fixes +F: Documentation/networking/device_drivers/ethernet/3com/vortex.rst +F: drivers/net/ethernet/3com/3c59x.c + +... + +THE REST +M: Linus Torvalds <torvalds@linux-foundation.org> +L: linux-kernel@vger.kernel.org +S: Buried alive in reporters +T: git git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git +F: * +F: */