Sorry to chime in here like that, as this may go a little bit of topic, but I feel you are missing a critical factor here:
Code review “works” at $WORK (henceforth just work) because people, or rather employees, are usually aligned towards common goals or principles, either by their own ambitions or by external forces, like management or customers.
My personal and very limited observation is that these common principles do not exist for nixpkgs or are not really followed up upon by leadership. Unless it comes down to the plumbing of the whole thing, everyone is pretty much free to push in whatever they wish, as long as they find someone to give them a thumbs up on their PR. This will not change simply by enforcing a four eyes principle or through automation. If you want to minimize friction for your PR, you will always be able to find someone who is willing to give you a drive-by approval. More automation will also not change that, as I don’t believe it will address the core problem in any way, it just strives to make the status quo more bearable.
So as long as the people involved in nixpkgs maintenance focus on achieving technical solutions (I count code-review as a technical solution here) for non-technical problems, things will not really improve, they will just appear less problematic for a time until the current “solution” is overwhelmed again.
So as long as the people involved in nixpkgs maintenance focus on achieving technical solutions (I count code-review as a technical solution here) for non-technical problems, things will not really improve, they will just appear less problematic for a time until the current “solution” is overwhelmed again.
Feel free do disagree
It is even more interesting to look for technical solutions while disagreeing what exactly the problem is, and possibly whether it even exists… Of course, such disagreement might also become a problem, or there can be a disagreement on whether it is.
I have to be honest, I don’t understand this at all. How can there be more total merge button pushes?
The merge button gets clicked the same number of times. Technically there is a touch more work involved, because now Tootie(a committer in this example) needs to say hey Tito(another committer) can you merge my stuff?
Likewise Tito needs to ask Tootie to commit her stuff. But really, if Tito and Tootie generally work on the same sorts of things, they can just pair off each other and Tootie knows to always look through for Tito’s PR’s, and vice versa, instead of just merging their own.
Eventually(and probably very quickly) these pairs will work themselves out.
This goes for ckampka’s note also.
That would not change, see above.
I don’t disagree that the scope changes at an OS level, versus a work product or via OSS vs paid work, But I think the thing here is, even if ckampka is some “10X developer genius type” that is off in the haystack re-ordering esoteric complexities that mere mortals can’t handle, chances are that still shouldn’t get committed until at least one other person understands it, etc.
So while I don’t necessarily disagree with your over-arching thesis, I disagree in practicality. Having another set of eyeballs for code that gets pushed into Nix/NixOS brings more understanding of the various bits and pieces that make up the OS, which overall makes the final product a much better one for everyone involved.
But it’s clearly not something some people seem to think is a very good idea, so we can drop it from discussion and get on with our lives. There is basically no chance that I’ll convince you so the discussion is mostly pointless no?
Also for the record, I never argued against pull request in principle or pull request as a measure to spread knowledge. However, neither is what this thread is about. It is about whether or not we can prevent controversial changes from just getting merged and in my opinion, there is simply no basis for assuming it would. The nixpkgs-committers pool is only growing with a decent chance that at some point you will actually know only a minority of the people involved. Outside of that minority, you have no idea what motivates people to commit to nixpkgs and what their standards look like. Chances are also that if you as someone from your peer group politely (or you simply know someone of IRC who you know shares your goals), they will simply fast-track your code without the controversial discussion even happening. Or, they might just take your side in the argument and just merge anyways.
Code review without alignment does not protect against recklessness, laziness or code getting merged despite disagreements.
Another aspect of this discussion is that the leadership group is relatively amorph. We have one BDFL. After that, some members of the community are more or less established. I am sure that the exact set will vary when asking different people.
I agree that even with all the tooling, there will always be possible venues for conflict to arise. If we had a small group of good leaders, they could also be invoked in those cases to split the decision.
I was also kind of surprised by this merge. I’m not opposed to the change, it’s probably the right decision, but…
AFAICS this case was a bit more subtle though: while someone commented ‘I don’t have strong feelings on the default value of this flag’, and that person was a nixpkgs maintainer, he was not listed as a maintainer for this particular package. Also, he only commented, he chose not to approve the PR.
Since there was some reluctance to the change, I think it would have been reasonable to wait for someone with merge rights to approve the PR before self-merging. As a maintainer without merge rights, I know how frustrating this can be, though .
What doesn’t help is that the person who is listed as maintainer for this package seems to be relatively inactive.
Agreed, if this PR was by the package maintainer then I think the merge should’ve been noncontroversial.
I would definatly agree on that. I feel that the NixOS/-pkgs community could really benefit from a more active, maybe Debian-esque leadership system that gives the whole project some sort of focus an direction. That would make things a lot smoother imo.
I would definatly agree on that. I feel that the NixOS/-pkgs community could really benefit from a more active, maybe Debian-esque leadership system that gives the whole project some sort of focus an direction. That would make things a lot smoother imo.
This would be a lot of restructuring. The entire RFC process was painfully constructed to put the judgement calls on larger design decisions onto people who pledge to look into the details of the arguments carefully this specific time on this specific topic, and avoid demanding the same people to read up infinite amount of nuances about too many things.
I don’t think those are contrary, I would argue that those are actually complementary.
Just because you strive for some general alignment, eg. “For 21.04, we want to focus on achiveing XXX” does not mean that you would not need a process to flesh out how to actually achieve this XXX, but it could defiantly help people prioritize where to focus their attention and effort.
Just because you strive for some general alignment, eg. “For 21.04, we want to focus on achiveing XXX” does not mean that you would not need a process to flesh out how to actually achieve this XXX, but it could defiantly help people prioritize where to focus their attention and effort.
Nix* community is historically much better at working together despite being divided on basic priorities than at resolving sometimes literally reverse order of priorities based on vastly differing usecases, preferences and contexts.
First, I am not sure this is really going to work in a largely volunteer project. If you appoint project leaders, who say “X is going to be the goal of 21.05”, who is actually going to put in their time?
What happens is whatever the individuals contributing to the project want to spend their time on plus what a small number of companies around the Nix ecosystem (e.g. Tweag and NumTide) contribute based on their (customer’s) needs.
I am also not convinced that Debian is a good example. We are doing better on a lot of metrics. More packages, more fresh packages, a far lower bar for contribution, quicker adoption of new technology, a single coherent repository rather than many disjoint repositories, etc.
At the risk of becoming to repetitive. I think currently our biggest problem is that we have a lot of packages combined with many inactive maintainers. A common pattern is that new people show up, they add the 5-10 packages that they need and then they disappear. The maintenance is then left to the people who stick around.
All of that is nice to have, but only if you can manage the workload that comes with it.
The Nix* community as it stands can not.
That is a pattern that is common for a lot of distributions and unavoidable for projects where people are contributing based on their intrinsic motivations alone. All volunteer-driven distributions have that issue, but most decide to deal with it before it becomes a problem, usually by moving those packages to an unmaintained space so they do not become a burden on others. The fact that Nix* chooses to address this the way it currently is is not a problem, its a choice.
I must admin, I sympathize with that a lot. On average, I wait about a month or more to get a review for a simple version bump reviewed, and that is with multi-channel begging for it. That gets you frustrated very quickly. Though I am still listed as a maintainer on a couple of packages, I cannot even in good conscience call those packages “maintained” because I cannot tell if a fix that I push in June will make the cut for the release in September. That basically already stops me from taking on anything else except from drive-by patches that I don’t want to maintain in overlays or that scratch some particular itch.
Which brings me back to the beginning of the discussion: Unless someone steps up and faces the blowback of cleaning up nixpkgs and get it into a workable state again, this whole situation will not improve on community consensus, at least I can’t imagine how it would.
Check out marvin. Its not a silver bullet of course, but it might help. It puts you in an active position: Currently the oldest unreviewed marvin PR is one month old. If you are frustrated by the lack of reviews, you can put on your reviewer pants and clear the queue yourself. That could be done by a single motivated person today.
Sorry, but I don’t see how it addresses any of the problems discussed in this thread.
To be clear, the fact that reviews take ages to get to is not the problem, it’s the symptom.
From what I understand of what the bot does, it will likely make matters worse, because
it’s main function is to annoy people and to hug attention to opt-in PRs that would otherwise
be available on a fair basis. So thank you, but it does not really look attractive to me.
I am not sure how appointing project leaders is going to help with this. You can set a goal “all open PRs should be triaged and reviewed for 21.05”, but if you do not have the manpower it’s not going to happen.
There have been proposals with relatively wide support that could reduce the stale PR problem by empowering maintainers, such as the RFC that proposed a merge bot that allows maintainers to merge PRs against their packages. The RFC submitter did not have the time to bring the RFC to completion. However, if someone could put in the time to revise the RFC, address the comments, and implement such a bot, that would result (IMO) in a large improvement in the nixpkgs workflow. It would also solve the frustration that you describe – that it is frustrating to be a maintainer if PRs of packages that you maintain never get merged.
Any way, I am going to stop here. Because we have drifted far of the original topic, plus it might more other people, since this discussion comes up very regularly .
I am not sure if we are drifting, because this RFC essentially brings us full circle, just by putting a robot in between. It will enable people to push their own changes through without having to address controversy or discussions. But yeah, lets stop this here
Coming back to topic: I think it would have been better to make a stronger attempt to sort this out privately or in a smaller circle before escalating the issue. Clearly nobody had bad intentions here. “Misuse of commit permissions” is a serious accusation, and I can only imagine how demotivating that must be. I have no strong opinion on the technical side, but I support @talyz in that sense.
Praise in public, criticize in private. If you want to discuss systematic issues, like self-merging, that can of course be discussed in public. But I suggest to do that with as little finger pointing as possible.
.
.