Add MAINTAINERS file + associated policy / tools #13602
Comments
Do I understand correctly, that H: line can also be repeated?
What do we do with PRs effectively creating new subareas? What do we do with tools/misc and similar? How can I mark a package «feel free to ping me for review/ask for For small things the key (in my obviously wrong opinion) thing we lack |
|
👍 |
|
|
👍 |
Please assign some more trusted reviewers for such things, we need all Adding things should have a lower risk of breaking everything,
Ah, so not all packages fall into such groups. Noted.
meta.reviewers, probably — there are maintainers who can help with |
|
👍
If I understand correctly, this is not about restricting the actual permissions of committers, but establishing policy and developing tools to notify the appropriate maintainers. At this point, I am comfortable trusting all the committers to follow an established policy; the only problem is that we don't as yet have one 😃 |
|
Oh! I misunderstood. I agree wholeheartedly with establishing policy! |
|
Yay! I returned again and again to a thought "how would I specify that I (or someone else) is responsible for this directory tree / feature / function?". This would scratch my itch. |
It could be nice to have brief recommendations for people with bigger For example, a few years ago GNU TLS updates loved breaking libsoup and |
|
Different proposal I think linux long flat file is nice, but maybe there's something better we can think nowadays, and different because of our nixpkgs layout. Also, consider that regexp are not much of any use in our tree because most of stuff is a single file in a directory.
I think this will also make us think about subtrees differently: more maintenance-wise than category-wise. Like, why do we have EDIT: slightly changed the header of the MAINT.md, but that's only a detail of this proposal. |
|
@lethalman Re: the compilers category, it might be a useful designation for packages which are known to gate many other packages and whose build failure should be taken more seriously when cutting releases. The overall oddity of the I agree that having the description of the subtree procedures sounds useful, as does not having everything in one monster file, just trying to note some potential complications to address beforehand. P.S.: I'm mostly a user, not a bigshot maintainer, so these are just my two cents, sorry if I'm bothering you folks. |
|
@maurer I don't see any issue in creating more |
|
There is a pretty decent semi-standard file format for exactly what we're trying to accomplish here: It's used in chromium's git repos: Another related implementation, as a Gerrit plugin: More related tooling: Some commentary about origins and reasoning: The net effect is comparable to the linux MAINTAINERS file, but this can be distributed throughout a codebase like @maurer's MAINT.md proposal. |
|
I don't think per-subdirectory files would work well in this specific case which was about cross-cutting concerns, which kind of tend to touch files all across the directory tree. For instance I'd be thinking of something like this for myself: ...which is already all over the place and directories like Also if we use the MAINTAINERS format from kernel, we can also steal their script (https://github.com/torvalds/linux/blob/master/scripts/get_maintainer.pl) which does other kinds of useful stuff already (like ignore files and using |
|
Can somebody give perspective on MAINTAINERS.md: 1,859 MAINTAINERS: 219,266 OWNERS.md: 323 OWNERS: 224,132 |
|
@lethalman What @maurer said. Some topics don't correspond to subtrees, in fact some may not correspond to any specific files at all. (E.g., "Nixpkgs coding style", "New packages / modules", ...) |
|
@edolstra I wonder what means being maintainer of nixpkgs coding style. |
|
Regarding existing tooling, we'll probably have to write/adapt our own anyway because it will need to be able to handle meta.maintainers in addition to MAINTAINERS/OWNERS. |
I like the general idea, though I wouldn't bind that file to the github infrastructure so closely, but allow email entries like so: So we can also be reached by non-github users via email. |
Add the name of maintainers.nix entry then. And display name. |
|
Wouldn't it be better to add a list of paths maintained to |
|
Why |
|
@jagajaga This was just an example. Of course it would be @matthiasbeyer and mail at beyermatthias [dot] de for me. |
|
To my mind, the assignment of ownership is what makes this proposal valuable. That there is ownership is more important than who owns what or how it is recorded. I hope this doesn't get bogged down by trying to hash out all the details up front ... What @edolstra proposes seems perfectly workable as-is to me, though @domenkozar's idea of re-using maintainers.nix makes a lot of sense, esp. for the long term. Just my 2 NOK. |
Will we keep the bot that looks up blame information and cc’s the responsible users then? I like having everyone interested in cc, but I’m afraid of creating too much noise in general. Another thing I loathe about the current system is that there is sometimes no connection between mail address, IRC user name and Github user name. |
You may want to run my vanity counter from maintainers/scripts and use |
Some time ago GitHub introduced the CODEOWNERS file. The file is similar to the MAINTAINERS file that was proposed in NixOS#13602. Code owners will automatically receive a review request.
|
Hm, what about running our own mention bot instead (we could even patch it to incorporate |
|
Yeah, a patched mentionbot that uses meta.maintainers would be ideal. |
|
Nitpick: then it would be a rather important requirement to enforce matching maintainer IDs with github account names or add some extra mapping for that. (For instance, Eelco's names don't match :) |
|
Maybe add optional fields like Nix-related GitHub username, Nix-related GitLab user name, etc.? (Yes, my |
|
Very relevant thoughts: http://blog.ffwll.ch/2017/08/github-why-cant-host-the-kernel.html |
|
So what's the next actionable thing to do given that the CODEOWNERS file was merged in #27499? |
|
Some support for |
|
As to the next actionable things:
|
|
I think there would be the downside that the feature can only be used if we disallow direct pushing, i.e. even noncritical changes would AFAIK have to go through PRs, which tends to be annoying in my experience for trivial/safe changes. (I use it in gitlab for my paid job, but the speed of changes is much slower in there.) |
My experience from "allow direct pushing" is "I can do this, I know my code works" ... except when it doesn't. So I'd opt for disallowing direct pushing everywhere, even for security patches (for example, I cannot push to master in my own projects, where I even work alone on the codebase - just because "My code always works" does simply not exist). But that's just my 2ct. |
|
Well, if we had some (more) reliable CI for PRs, that would be a different question... (But even there – it feels strange to wait for functional tests when you only want to fix a typo in docs.) |
|
Yet different question if there were not a huge backlog of PRs…
|
|
|
|
@vcunat Regarding the typo fixes, one could look at how rust-lang/rust does it. They have two kinds of patches, 'real' patches and fixups. Fixups get batched and periodically run, removing pressure for such changes. |
That's indeed a good idea! I would even include smaller package updates into such a branch (like if |
|
Erm. I do actually care about some of the leaf-package updates specifically at the moment I perform them. |
|
We actually batch the "larger updates", as they are often mass rebuilds :-) EDIT: I suppose Rust's motivation is partially similar and they have some extensive test suite which isn't economical to run for each small fix. |
This now exists, and is used. |
|
Closing in preference of NixOS/rfcs#19. |
|
This issue has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/missing-documentation-of-makeautostartitem/20496/2 |
Following up on http://lists.science.uu.nl/pipermail/nix-dev/2016-February/019811.html.
To prevent the Nixpkgs from descending into chaos, we need some policy about who is allowed to commit/merge what. We already have meta.maintainers for packages, but not for other parts of Nixpkgs/NixOS. In particular it's completely unclear who "owns" central parts like
lib/so it risks becoming a free-for-all.So I would like to propose the following:
Add a MAINTAINERS file (in the style of https://www.kernel.org/doc/linux/MAINTAINERS) listing who is responsible for particular topics / subsystems. This would generally only list things that cannot be expressed via the meta.maintainers field in packages or modules, such as cross-cutting concerns or general responsibility for a set of related packages like KDE. Example entry would be
where "H:" is a GitHub user name and "F:" are filename-matching regexps to help identify whether a particular change affects this subsystem (not required).
Add meta.maintainers fields to NixOS modules.
Document the policy (e.g. changes need approval from the responsible maintainers).
Make a little tool that given a diff tells you who the responsible maintainers are. It would do this by 1) matching the modified files in the diff against the "F:" regexps in the MAINTAINERS file; and 2) looking for a meta.maintainers field in the modified files. Also make a GitHub bot that applies this tool to incoming PRs and pings/assigns the maintainers.
Does this sound reasonable?
The text was updated successfully, but these errors were encountered: