nixpkgs
nixpkgs copied to clipboard
NixOS
New nixpkgs committers requests
This issue is used to nominate people for commit rights instead, scroll to the bottom to see how it's done.
Nixpkgs has an ever growing number of PR's but not enough people to review and merge them. We need more nixpkgs committers! But we can't just select a random bunch of people and give them commit rights. It is a matter of trusting them to keep the codebase clean. People should go through a review process to be able to become a committer. This issue should serve as a place to discuss such a process.
https://github.com/orgs/NixOS/teams/nixpkgs-committers/members
Having discussed this on IRC a bit, this is an arbitrary set of requirements I came up with:
To become a nixpkgs committer you need:
- To have at least 50 merged PR's, 10 of which non-trivial
- To have review at least 20 non-trivial PR's
- To have added at least 2 new packages, to know how to write nix code and package stuff in nixpkgs
- To have written at least 1 NixOS module
Of course, a manual check will be involved as well, but having such a set of requirements could set a baseline.
In addition, a series of interview questions could be asked, such as:
- "How does master and staging work, when to use what?"
- "Describe the release process, what do you need to look out for during the release time?"
- "There is a PR that updates libfoo, how could you test this PR?"
- "There is a PR that adds 1000 NixOS options, what are potential problems with this?"
- "A PR breaks backwards compatibility, what influences your decision on what to do?"
Having discussed this on IRC a bit, this is an arbitrary set of requirements I came up with:
To become a nixpkgs committer you need:
- To have at least 50 merged PR's, 10 of which non-trivial
Do you want to add that of last 20 PRs submitted there shouldn't be major concerns about code quality during review? Controversial changes being rejected are OK, though.
Maybe 50 is slightly too much, and also maybe we want to have some preference about recency (do we want a track record of at least two months? and do we want at least 20 accepted PRs to be less than one year old?)
- To have review at least 20 non-trivial PR's
I have some vague feeling that this is a risky guideline, maybe we just want to lower the number. Basically, we want non-committers to review things where they are already confident, and some part of a person's project participation time should have gone into getting that confidence. Maybe 20 is a large enough number that it looks like we expect people to be a bit more aggressive about what they review. Not sure (as I said, this is a vague feeling).
- To have added at least 2 new packages, to know how to write nix code and package stuff in nixpkgs
I would say explicitly that a notable refactoring of large packages also counts. If there is a place where we can say that we value cleanups, we should say it.
Refactorings are normally reviewed in a stricter way than additions anyway.
- To have written at least 1 NixOS module
Not sure I agree with that — I think having more Nix-on-non-NixOS committers is a good idea. I think just asking people not to touch NixOS modules until they have done some changes via PRs (merged by experienced people) should be OK.
Of course, a manual check will be involved as well, but having such a set of requirements could set a baseline.
I am not sure what would be in the manual check as a separate consideration. Obviously, there are a lot os subjective value calls in the guidelines — non-triviality, reasonableness of reviews, code quality problems in recent commits, etc.
Exceptions from the baseline guidelines are another story, of course.
Given how things work now, I am not sure what would be a reasonable way to conduct such an interview.
- "Describe the release process, what do you need to look out for during the release time?"
Do many correct answers start with «there is no fixed release process, but generally…»?
Actually, recognition that Nix* is very slow to achieve a consensus on long-term decisions could be a good thing to check…
A question I don't have a good answer for: should we somehow split up "people who are keeping things up-to-date" vs. "people who we trust to refactor/make sweeping changes"? I'm very much in the set of people that use NixOS, but don't have any strong feelings on how nixpkgs is laid out, what the future direction is, etc., but am happy to submit bugfixes (e.g #47255), closure size reductions (e.g #49315) and security updates (e.g #49859, #49860). I have no immediate interest in doing any refactoring of nixpkgs, but would happily use any additional permissions to continue "just fixing things", like above.
A question I don't have a good answer for: should we somehow split up "people who are keeping things up-to-date" vs. "people who we trust to refactor/make sweeping changes"?
Do we have any project member who hasn't submitted PRs because the changes were indeed global enough to discuss? I doubt that.
I think making and following reasonable estimates of desired review levels for each change is what we target.
Right, sorry, maybe I’m not being clear here. To be super blunt: y’all should (rightly) not trust me / people in my situation to submit arbitrary PRs, but if you could somehow ensure that the only thing we were doing was updating packages (or other non-controversial/scary changes), maybe it would take some load off the trusted reviewers and let them review things that mattered more?
Again: just thinking out loud here, so to speak 🙂
On Sat, Nov 10, 2018 at 1:54 AM Michael Raskin [email protected] wrote:
A question I don't have a good answer for: should we somehow split up "people who are keeping things up-to-date" vs. "people who we trust to refactor/make sweeping changes"?
Do we have any project member who hasn't submitted PRs because the changes were indeed global enough to discuss? I doubt that.
I think making and following reasonable estimates of desired review levels for each change is what we target.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/NixOS/nixpkgs/issues/50105#issuecomment-437572238, or mute the thread https://github.com/notifications/unsubscribe-auth/ABB3hSemV6gEn2gSqOxzMkHc5iTli9Grks5utqJtgaJpZM4YXx1N .
but if you could somehow ensure that the only thing we were doing was updating packages (or other non-controversial/scary changes)
https://github.com/kragniz/nixbot
People with commit access are covering many roles that are more or less evident: they reduce the queue of unmerged PRs but at the same time they ask for or make changes (thus contributing to the overall quality) and most of all they are shaping nixpkgs layout and functionality by boosting or closing some PRs. @Infinisil it looks like those guidelines could be a rule of thumb to what can be called a nixpkgs core team, much like the Nix core team but applied to nixpkgs. At this point we have some people with commit access and, knowing almost nothing of the project history, I can assume they earned it through trust and reputation so there is a process already but it's slow. So are these guidelines a way to measure contributions and speed up the process?
At this point we have some people with commit access and, knowing almost nothing of the project history, I can assume they earned it through trust and reputation so there is a process already but it's slow. So are these guidelines a way to measure contributions and speed up the process?
I think these guidelines don't differ much from when many people get commit access nowadays. Having a published set of guidelines would hopefully reduce uncertainty (and maybe even reduce selection for willingness to ask out of the blue — which sometimes holds some people out of the committer list without a good reason).
I don't know how easy it is to set up on github, but one way to allow less experienced people to help out with pull requests would be to create an intermediate tier of contributors who don't yet have full commit access, but have the ability to give a PR their approval. Once a PR has been approved by two or three of these 'semi-trusted contributors', it can be automatically committed by a bot.
Once a PR has been approved by two or three of these 'semi-trusted contributors', it can be automatically committed by a bot.
I think once ofBorg — https://github.com/NixOS/ofBorg/ — gets some auto-merge support, it will be reasonably easy to add a few kinds of advanced access control logic. There are tickets about that…
@Infinisil thank you for starting this process. I've been wondering for a while what are the requirements for me to gain push access and to be a part of the core team to help out with any refactoring or core decisions.
Over the past couple of months, I've submitted a bit over 50 PRs, some are version updates but many are not trivial (such as #47407, #49884 and #49815 among others).
I'd love to be the guinea pig of this process. @Infinisil @domenkozar please let me know if I would qualify for the minimal requirements and I'd be happy to jump with you (and anyone else from the team) on a call or chat.
Is the team taking names for consideration now? If there is still the desire to find help I wouldn't mind throwing my name into the pool of people interested.
What I would like to know is what members intend to do, that is, what parts of Nixpkgs do they want to work on, and what do they want to be responsible for. The repository is large, and it's close to impossible for anyone to keep up to speed with everything that's happening inside it. While we can add more people to merge changes, and keep adding more packages, there are various bottlenecks that need to be addressed.
An important part is also, what are our expectations of Nixpkgs? Aside from the stable releases, many users use Nixpkgs as a rolling release. A rolling release requires far more effort to keep a working package set. I'm hijacking the thread here a bit, but simply saying "we need more people to review and merge" is a bit short-sighted. While I would like to have more people involved, I think it's more important that we come up with a strategy for Nixpkgs, and how we deal with Nixpkgs members is going to depend on and be a part of that strategy.
I think there are a few issues to tackle here, but they are not so much related.
Nixpkgs has an ever growing number of PR's but not enough people to review and merge them
That's a problem we have to solve. So far, to-be maintainers were nominated on IRC and we agreed upon which to give access. I think the process as such is not perfect, but fixing the stated problem is more about making the process of nomination more explicit, known and transparent. I wouldn't impose any kinds of limits how to gain access (I remember how terrible Gentoo process was with this quizz). I think it's better to leave the judgement to existing contributors rather than come up with some unified scheme of scoring that can be gamed.
But we can't just select a random bunch of people and give them commit rights. It is a matter of trusting them to keep the codebase clean. People should go through a review process to be able to become a committer. This issue should serve as a place to discuss such a process.
It's not random, we so far delegated the judgement to existing commmiters and that worked pretty well.
I think we need:
- a form to submit that will nominate a potential committer
- have a way to discuss the nomination (github teams are pretty cool here since we can limit discussion only to committers)
- have more documentation about what is the responsibility of the commiter and a nice welcoming text
A completely different discussion is changing the structure of the current maintainer flat structure into something more organized.
@domenkozar Re: gaming — well, there is also a value in something to help with the impostor syndrome and to suggest to people when it makes sense to self-nominate.
Better documentation of expectations from committers is also nice, of course.
Nixpkgs has an ever growing number of PR's but not enough people to review and merge them
The solution to this is to modularize (split) Nixpkgs. For example, most NixOS modules should be maintained in separate repositories (== "flakes"). Then we don't need to have hundreds of direct committers to a huge mono-repository. Of course this will also scale much better in terms of Nixpkgs download time, evaluation speed/memory use, etc.
(read the bottom of the message first)
The solution to this is to modularize (split) Nixpkgs. For example, most NixOS modules should be maintained in separate repositories (== "flakes").
Of all the problems NixOS has, I don't think maintainership of modules is an issue. IMHO, the main issues are:
-
general "unwillingness" to test NixOS changes by the community, some fairly simple changes sometimes take years to merge because nobody is willing merge them untested, but accumulating a critical mass of testing requires years without merging to master where most people will see them
(IMHO, this can only get worse with "flakes", as now you would have to follow and harmonize several distinct repositories to make some high-level feature work.)
-
I think the "unwillingness" at least in part stems from poor discoverability of PRs on GitHub, i.e. "How do I find NixOS PRs in a mergeable state that might interest me?"
(IMHO, again, this can only get worse with "flakes".) (IMHO, Nixpkgs is just too big for GitHub.)
-
a particularly important case of discoverability is a mention bot or a similar system that should allow linking between related PRs. Clearly, the most likely parties to be interested in the change are those who made related changes before. It seems strange to me that nobody except me seems to be particularly frustrated by the lack of a mention bot. I got so frustrated that I actually made a similar/related thing myself and will start using it from the next PR I make, just wait :]
I do agree, however, that modularizing in the Linux sense, i.e. multiple repositories with a common tree, is a good idea. Like, for instance, having all package updates go into a separate GitHub repo would be nice. But GitHub, AFAIK, doesn't support that workflow well (like PR numbers would get all messed up between repos by default unless some conscious effort is given to clearly mark them with correct prefixes).
Also, currently it is very hard to subscribe to "common NixOS infrastructure, not any particular service" based just on options and paths, e.g. some services live in "config", some in "hardware", some in other random places. The option names they define are pretty much random. E.g., consider PulseAudio: it lives in "/nixos/config", defines "hardware.pulseaudio", while clearly implementing a service. Cleaning that up would be good.
An immediately applicable measures that, I think, would improve things for NixOS:
- make a "nixos-next" branch (like "staging-next" that should be called just "next") for "should be good, but needs testing" changes, merge it only when all nixos tests pass,
(I would build my laptop against it most of the time, I do that against "staging" and "staging-next" from time to time,
doCheckByDefaulthelps with that.) - cleanup nixos modules so that one could sanely use
configure.nixoptions and (even more importantly) repository paths as filters (e.g. for CODEOWNERS) for nixos.
I don't think NixOS has any shortage of people with commit bits. NixOS organisation has 65 members, plus, the nixpkgs repository has some additional committers that are not members. Looking at the list of members, I don't recall ~half of the names to ever merge a PR I looked at. Out of the remaining "half", a ~half would be mostly working on infrastructure and/or mostly merge their own PR or commit directly (I don't say they are not doing an important job, but in this issue we talk about reviewing PR's). My point is that putting work into reviewing those piles of PR's is not a small endeavour. In my opinion, the people who are willing to do that for free should be valued at a price of gold in the community. It will not be easy to find more people to help with that kind of work. I believe that just increasing number of people with commit privilege is unlikely to help the situation - it was not so effective before and I don't see any indication that it will help in the future. A more effective solution might have to do with improving the nixpkgs infrastructure to improve efficiency and convenience for people who are willing to work on reviewing. I don't know what exactly needs to be done. It could be improving some CI capabilities of @GrahamcOfBorg or adding functionality similar to Rust community's @bors or perhaps even gamifying the process a little bit (e.g. @rust-highfive).
@veprbl The thing is, people might get commit access at some point because they have a lot of time and motivation to contribute, and that is a great help at that time. But times change, people change, they might get inactive or change their field. Those are the inactive people with commit access, most of them contributed well at some point.
We also discussed this a bit on IRC on how to fix this: People no longer active in the Nix community should be taken away commit rights after some amount of inactivity. Specifically, after 1 year of not participating in any nixpkgs issues/PRs this could happen, people seemed to agree on this.
@Infinisil The number of members doesn't bother me at all and I'm not calling for purges of any kind. That was not my point. I'm just skeptical about your proposed extensive solution to increase the number of maintainers to solve the problem of PR pileup. I think there are some unexplored intensive measures that we could take.
@veprbl Well I'm certainly up for some purging though! I understand what you mean, but I disagree. Let's look at it like this:
- We have a certain number of people with commit rights
- Over time, some of these people become inactive
- This means, over time we have less and less active committers
- In order to prevent this, we need to add more new active committers than current committers become inactive
- Who can become an active committer? Active nixpkgs people, what this issue is about
- How can they become a committer? By knowing how to get commit rights, what this issue is also about
I think these 2 questions have long been standing in the way of many people trying to think of becoming an active committer. People don't know what "active" means and whether they are eligible. And people don't know how to ask for commit rights.
How did I get commit rights? After asking around a lot on IRC, I got the answer that I should ask @edolstra directly. So I asked him directly in IRC, and he was kind enough to give me these rights. Yes it worked, but it took me way too long to figure this out, and not everybody can figure this out, it's written down nowhere.
And yes, as seen in this issue, there are people who want commit rights, I don't think we'll have trouble finding them. On top of my mind, I know that both @worldofpeace and @kalbasit are relatively new community members both helping out a lot (thanks!) and wishing to get commit rights to be able to help out even more, and I'm sure there's more people to come in the future, this project is growing a lot after all!
I think this issue should stay a place to discuss how we can make the process of getting new committers easier and more standardized, not whether we should. We need to get new committers, there's no way around it, and this project wouldn't be this far if we didn't add new committers in the past. As a datapoint, @edolstra made me a committer 4 months ago and I merged 236 pull requests since then. I couldn't have helped out this much if I weren't a committer.
Re: splitting the repository: I remember NixOS being in a separate repository. But then some changes needed to be applied to both repos in sync, which lead to eventual migration of NixOS into Nixpkgs repository. I would expect spinning out overlays out of Nixpkgs to cause similar problems.
One of the things that has made Nix appealing to me (and easy to contribute to) so far for me has been that everything lives in one big repository. It means that there is one single directory I can grep through when I'm trying to figure out how something works, and one repository that I can bisect really easily when something has broken. Any time I've tried to figure out how something works on any other free operating system, I've become lost in a sea of repositories, where there's no way to reconcile which revisions match up, and to figure out what was used to build a system. For me, the monorepo has been one of NixOS's biggest strengths.
On the topic of new contributors, from my perspective as somebody who hopes to (at some point) become a Nixpkgs committer, and who has previously been one for another large package manager (Homebrew):
-
"To have review at least 20 non-trivial PR's" – as a non-committer to a free software project, it's often not clear to me whether reviews from non-committers are welcome. It can feel like there's not really any reason for my feedback to be needed, because it's not going to be up to me to decide whether to merge. Maybe I'm just going to be annoying people and getting in the way.
I've been around Nixpkgs enough to know that, in this community, that's not the case, but in general, when coming to a new project I might like to some day be a part of, that's not clear to me. And if that's part of the path to becoming a committer, it should be stated that this type of feedback from non-committers is welcomed, so that people don't miss out on that by not realising it.
-
I would be wary of over-formalising. If you have an entirely numeric public set of benchmarks for when somebody can become (or be considered to become) a contributor, it would be very easy to end up with a situation where there's somebody that the team feels, for whatever reason, wouldn't be a good fit, but has met the benchmarks, and feels that they aren't getting something that they have earned and deserve. On the other hand, those same benchmarks might slow down the process of bringing somebody on board who would unquestionably be an asset. For example, somebody who had made 300 perfect package update PRs and reviewed 300 more from other people would likely be very valuable, even if they had no interest in larger changes or NixOS modules or whatever.
-
When people do get commit access, it would be nice for the community to know a little bit about them. I think it would help build the human connection between everybody working on this software, and would be a really nice welcome for the new person. Maybe a "welcome X to nixpkgs" post on Discourse with a little bit about what work they'd been doing and why the rest of the team decided to offer the commit access.
I think there's a middle ground between hard rules and giving commit access to random people. In my mind, I see a flow where maybe 1 reviewer notices a person who has been doing a lot of good work, writes up a private post to the other committers about whether it would be a good idea to bring them on board, and then invites them in if there were no real objections after a certain amount of time would be a perfectly healthy way to grow the community that wouldn't be vulnerable to getting caught up in bureaucracy.
One more thing: if commit rights were something that had to be requested, I think that might put people off. "What if I'm rejected?", "What if it's rude to ask, and I should wait until I'm invited?". People can be shy or nervous, and it's important to remember that there is a power imbalance here between the "applicant" and the existing committers that could make asking straight up feel daunting.
I think it would be reasonable to assume that anybody with a sustained pattern of contributions would be at least interested in commit rights, and should be offered them when the team thinks it's appropriate. They can always decline. But that way, it's not up to one non-committer to work up the courage to ask.
- "To have review at least 20 non-trivial PR's" – as a non-committer to a free software project, it's often not clear to me whether reviews from non-committers are welcome.
I think right now we don't make it clear enough they are. I think a public document that says we expect new committers to have already reviewed PRs would be a part of making it clear. I have also expressed my doubt about the balance of numbers.
- I would be wary of over-formalising. If you have an entirely numeric public set of benchmarks for when somebody can become (or be considered to become) a contributor, it would be very easy to end up with a situation where there's somebody that the team feels, for whatever reason, wouldn't be a good fit, but has met the benchmarks, and feels that they aren't getting something that they have earned and deserve. On the other hand, those same benchmarks might slow down the process of bringing somebody on board who would unquestionably be an asset.
I do hope that people who are much better than me in writing texts for humans will succeed in positioning guidelines in a proper way. These should eventually be «typical expected experience level around the time of receiving commit access» or something like that.
I do not expect any problems with positive exceptions, although these will probably still take more time than the streamlined main process.
Negative exceptions are harder, of course; in some sense, I expect them to be predictable early in advance (probably before 20 accepted PRs), but no idea what is the best way to handle this proactively.
- When people do get commit access, it would be nice for the community to know a little bit about them. I think it would help build the human connection between everybody working on this software, and would be a really nice welcome for the new person. Maybe a "welcome X to nixpkgs" post on Discourse with a little bit about what work they'd been doing and why the rest of the team decided to offer the commit access.
I feel «welcome X to Nixpkgs» has unfortunate implications about non-committers, more like «welcome X to Nixpkgs committers».
I have an expectation that «why the rest of team has decided» might encourage writing some paragraph in exact same words in almost every post like that.
I think regardless of human connection, knowing who is interested in what technical areas is very useful, and we seem to be beyond the point of being able to keep track of that without technical aids.
I think it would be reasonable to assume that anybody with a sustained pattern of contributions would be at least interested in commit rights, and should be offered them when the team thinks it's appropriate. They can always decline. But that way, it's not up to one non-committer to work up the courage to ask.
On the other hand, if we want the process not to be blocked on the community-focused people in the team having or not having enough time, we need to make sure that asking for commit access is a normal thing that people do (and succeed). I hope guidelines will help with impostor syndrome (if you meet the guidelines, yes you do have enough experience and no nobody can keep track of everything in Nixpkgs anyway). But quietly doing useful work in the neglected corners makes a person harder to notice — and at the same time, it makes the same person valuable in an irreplaceable way.
And people who apply might have an easier time to remember some examples of their less straitforward contributions. Trying to quickly review contributions of many people to find out who has been missed is tiring (I know, I have tried to do that systematically but gave up after doing it a few times and talking a few people into succesfully applying). Keeping notes between the attempts could be helpful, I guess, but then we might end up with a leaderboard for a race towards commit access… Maybe at least a non-public one.
Or maybe joining this with the «interested in …» list could lead to a wiki about community members with areas of current interest and links to recent examples of things people participated in; that could help with both finding people to ask for advice/review, and with looking for people missed by commit access offers.
Basically, every process that requires regular active involvement from existing committers has a risk of stalling, and if we make the default to be «offering the access», people who are missed might think they just need to wait.
I am very much in favor of an approval system + automatic merge by ofBorg because i personally do not want to have direct commit access to nixpkgs master but i actually do like to help out with PRs by performing reviews. For me pushing directly to the branches of the repo is a super scary thought and having some automatic entity which actually performs the merge gives an ease of mind.
Right now what i am doing is occasionally is reviewing changes and use the approve functionality by github to mark PRs as "lgtm". This makes it easier for certain maintainers i know personally (e.g. @Mic92 ) to actually perform a quick check and merge.
For me pushing directly to the branches of the repo is a super scary thought
Personally, I disabled pushing to origin with git remote set-url --push origin no_push that way I cannot make a mistake and push to the wrong fork.