Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How will you help decrease the number of open PRs in Nixpkgs? #84

Open
PedroHLC opened this issue Sep 27, 2024 · 7 comments
Open

How will you help decrease the number of open PRs in Nixpkgs? #84

PedroHLC opened this issue Sep 27, 2024 · 7 comments
Labels
question Further information is requested

Comments

@PedroHLC
Copy link
Member

PedroHLC commented Sep 27, 2024

Question

With the whole Wayland-protocols debate going on, one cannot skip the parallel to Nixpkgs. Some users have a good friend committer to have their PRs reviewed and merged. But the frustration even hits some committers that sometimes are caught self-merging.

Practical solutions to end the frustration would (and will) take the entire community participation. How can you, as a lead, help both to accommodate all the opinions and to speed the adoption of solutions, taking down the number while caring about the health (consider burnout) of those involved on applying those measures?

Candidates I'd like to get an answer from

No response

Reminder of the Q&A rules

I'm following and will keep following the Q&A guidelines and rules

PS: Good job on everything related to this repo!

@PedroHLC PedroHLC added the question Further information is requested label Sep 27, 2024
@PedroHLC PedroHLC changed the title [Title] How will you help decrease the number of open PRs in Nixpkgs? How will you help decrease the number of open PRs in Nixpkgs? Sep 27, 2024
@mschwaig
Copy link
Member

I think one productive way to make sure large parts of the community are involved in solving this issue, and to reduce the pressure on individual heroic reviewers, would be spending time and effort on designing incentives that give us the results we want.

For example:
There are just as many contributors waiting to get their changes merged, as their are open pull requests (well, except for the bots).
How can we incentivize those contributors into reviewing the PRs of others and do it diligently - in order to get their own PRs reviewed and merged faster?

For sure any such proposal would be controversial to some degree, and a lot of discussion will happen in the community and with the SC. In the end I see the role of the SC as taking responsibility for this kind of decision.
When there is a specific proposal on the table that people have worked out and analyzed, the SC can make the decision to try it.

@nyabinary
Copy link
Contributor

nyabinary commented Sep 28, 2024

To address the challenge of reducing the number of open PRs in Nixpkgs, I would advocate for more automation within Nixpkgs, with a particular emphasis on expanding the capabilities of OfBorg. By automating more aspects of the package management process, we can significantly reduce the manual toil/workload on maintainers and committers, which is, in my opinion, critical for preventing burnout and retaining the bulk of our contributors. I will also push for the implementation of automated bumps for a carefully curated core set of packages, ensuring these packages remain consistently up-to-date with minimal manual input. These packages will be supported by a suite of automated test (including unit, integration, regression, and performance test) further decreasing the manual effort required from reviewers and allowing them to focus on more fulfilling tasks than mind-numbingly tasks that require lots of tedious manual labor.

Furthermore, I recognize that many in the Nix community find it challenging to contribute back to the Nix ecosystem, according to recent Nix community surveys results. I will advocate for clearing away roadblocks that make it difficult to contribute, ensuring that the barriers to entry are minimized and that new contributors can more easily participate in the ecosystem. This will involve streamlining processes, improving documentation, and providing better onboarding resources. Additionally, I will actively seek feedback from the community on specific pain points and work collaboratively to address these issues. By making it easier for people to contribute, we can foster a more vibrant and inclusive community, ultimately strengthening the entire Nix ecosystem as a whole :).

@illustris
Copy link

My thoughts on this are of course biased by my own frustration in getting PRs merged. These are some possible solutions to reduce frustration and improve the contribution process:

Replace nitpick reviews with automation

The feedback loop for reviews that suggest small syntax changes is too long. It is wasting both the contributors' and reviewers' time. We can handle these with tools like nixf-tidy, leaving reviewers to concentrate on more substantive feedback.

Define guidelines for PR reviews

It's essential to define what makes a PR "merge-ready." Reviewers should avoid delaying merges over trivial issues, especially for PRs that have been open for a long time and have undergone multiple rounds of minor feedback. We should develop guidelines that outline what a constructive review looks like, focusing on significant improvements rather than perfection. Reviewers should communicate clearly about the necessary changes for a PR to be accepted and set appropriate expectations, especially for larger contributions that may require multiple review cycles.

As a member of the steering committee, I would:

  1. Promote more automation in the review process
  2. Advocate for developing review guidelines
  3. Foster an environment where contributors feel valued and supported

@tomberek
Copy link

Our values include focusing on automation, and that is an obvious way to mitigate the problem. There are efforts such as the nixpkgs-merge-bot that can hep here as well as better contribution guidelines and onboarding additional contributors.

A specific thing I would like to start a discussion about is the lack of formal Nixpkgs teams. The largest and most valuable portion of our ecosystem is Nixpkgs, and is has no clear ownership as a whole or in for many sub-areas. This also means that Nixpkgs contributors often have a less clear voice into the Foundation and organizational matters, when they should. It also means they have less access to resources. Due to much package management being non-conflicting, a lot of Nixpkgs does not need formal ownership by established teams (a good example of "distribute decsionmaking widely"), but certain core areas and people do serve as an escalation mechanism for when things become stalled, or need a decision, or to provide a technical conflict resolution. There are efforts to be more clear about ownership in the community. One can see a few teams that have already naturally coalesced, "staging", "darwin", "compilers", "pythonPackages", "CUDA" and so forth. The exact breakdown should be fluid, but the overall idea is to encourage self-organized groups to be identified and provide them with the support they need.

This creates a situation where PRs have a clearer escalation mechanism if they are stalled, provides ownership (both authority and responsibility) to manage the problem, and helps break it down into a more manageable problem.

@getchoo
Copy link
Member

getchoo commented Oct 2, 2024

Similar to many of the other responses here, I think the best solution in the long term for new PRs is automation. As a maintainer of ~70 packages, nixpkgs-merge-bot has been a godsend in getting updates for my packages into nixpkgs and I would love to see more work on this specifically to remove some of its limitations (like https://www,github.com/NixOS/nixpkgs-merge-bot/issues/95). As a somewhat frequent reviewer, I have greatly appreciated the work done to take some of the weight off my shoulders, such as https://github.com/NixOS/nixpkgs-vet, the introduction of nixf-tidy into CI, and QoL additions that save me some time waiting on Ofborg. Reiterating a previous answer of mine, I think it's important to encourage and assist contributors in taking advantage of our testing infrastructure and passthru.updateScript in order for all of these tools to be much more effective and avoid some of the errors that may come from automation as best as possible

Giving more power to non-committers is also a concept we should really continue with. One example I've seen recently that I believe has a lot of promise is allowing for people without push access to be CODEOWNERS, which would hopefully allow for faster (manual) reviews by ensuring those responsible for certain files are always pinged regardless of committer status

Lastly, introducing more teams for more specific categories of nixpkgs would hopefully drive more attention to said categories from a group of people that have the time for and are passionate about them. As said by tomberek above, this has already developed naturally in some form, but should be expanded and made much more explicit. I also believe in the creation of a team dedicated to being a point of escalation for stalled PRs across the repository in the event of the aforementioned methods failing

@cafkafk
Copy link
Member

cafkafk commented Oct 6, 2024

I'd prefer to focus on the open PRs part of this and avoid the Wayland protocol
debate. Also this answer is just for nixpkgs. And... I should start by saying
that nixpkgs is one of the most productive projects on GitHub, so it is doing a
lot of things right. But there is room for improvements.

Nixpkgs has several issues that increase the time it takes for a PR to get
merged, here are some I think are particularly actionable:

  • Slow CI
  • Unclear guidelines
  • Discoverability problems
  • Lack of ownership
  • Commit bits are binary
  • Review Culture

Slow Ci

Currently, the CI for nixpkgs catches a lot of errors that makes a PR
unacceptable, which saves a large part of the maintainer burden associated with
reviews. The problem is that certain CI steps, particularly those related to
passthru.tests, and even more so the MacOS ones have a lot of problems.

One thing that massively slows down a lot of merges is the lack of clarity
around CI. Should you wait for the MacOS runners, even if they've taken several
days now? Are they currently broken?

This problem is exacerbated by unclear guidelines around CI. While it's
highly practical that veteran committers are able to merge without waiting for
all tests in case of security issues for instance, the common version bump or
minor refactor can get stuck in limbo.

Another problem that arises from unclear CI guidelines is that when people end
up merging something (whether self merge or after a review), and there are CI
steps that have yet to be completed, even if those steps may not have been worth
waiting for (e.g. they ran the MacOS passthru.tests locally), other committers
may still scold them for doing so. I've observed this leading to a cooling
effect, where many seem scared to merge until they're absolutely sure not just
of the quality of the PR, but also that there is no way for others to criticize
them for doing so.

Unclear guidelines

In the same vein as above, nixpkgs in general suffers from a problem of unclear
guidelines. This results not just in slow merges, but has even lead to
contributors leaving the project.

While a lot of the expectations for a good PR are codified in various markdown
files, they all have slight discoverability problems. Also, there is no single
centralized checklist that you can follow to ensure that a given PR lives up to
formal standards.

This leads to a situation where a lot is left up to the individual reviewer, and
while that flexibility can be useful in many situations, it can also leads to
cases where various nitpicking will stall a review, and grind down the
contributors.

One of the points of guidelines and CI checks is that they reduce or eliminate
nitpicking. The stronger the guidelines, the less is left up for debate. This
also benefits the people with a particular axe to grind, instead of having to
fight for your nitpick in every single review, having a centralized shared
rule-set means that you just have to reach community consensus once, and get it
added.

Better guidelines will also mean that a contributor can ensure that their PR
will cause as little friction as possible. The ideal state is one where if
you've lived up to all the codified expectations of the project, your PR will be
approved without needing changes. Every round trip costs a significant amount of
effort for both parties, and at any point either party may become too tired of
the process that they give up, and all of that effort is lost.

Discoverability problems

Often times, matching a reviewer with a PR that is of interests to them is done
through the CODEOWNERS file. While this is very efficient for parts of the
codebase that are critical, a less restrictive version of this solution would be
useful for maintainers. While ofborg will do a best effort to ping maintainers
on their packages, wider areas of interest can often go undiscovered. I've
personally had a few times where I've just stumbled on a kuberenets PR that
solved a problem I was highly interested in, and wished there was a mechanism
for being pinged about anything kubernetes related.

In many other projects, it's possible to simply watch a repository and just
ignore unimportant notifications, but for distributions, and specially those at
the scale of nixpkgs, that is way too much information.

Solving this by dividing into topics would be a huge improvement for matching
contributors, and it's somewhat the solution you see other projects like the
Linux kernel employ.

There are currently attempts at this being worked on, but I think they still
could be improved drastically, and heavily speed up nixpkgs development.

Lack of ownership

In cases where someone has maintained a package, module, or "subsystem", and
will continue to do so for the foreseeable future, it's important to give those
people some authority to decide on the direction of the things they maintain.

Drive-by reviews with various expectations of additions, scope creep, and added
friction often just increases the load places on these maintainers. It should be
the case that for many of the decisions about the direction of a part of
nixpkgs, the people that have to maintain those should get a larger say in how
they're run.

In eza we have a similar concept of committers privilege. The person that
commits the code gets to decide what direction to go if there is bikeshedding,
and CODEOWNERS have a larger say about the parts they own. This helps reduce
friction over time, gives ownership of the people actually doing the work, and
leaves them more motivated to work on the parts of the code they own.

Basically, by giving ownership to contributors, you ensure they're not alienated
from the things they work on. There is nothing more morale reducing than feeling
forced to work on problems you don't care about.

Commit bits are binary

A problem that has been attempted to address for a while is that currently, the
commit bits lack granularity. The community has been working on initiatives such
as the merge bot, massively reducing the friction of merging package bumps for
maintainers doing simple version updates.

However there is still a lot of room for improvement. Delegating and giving more
people the ability to be self sufficient would be a massive improvement. Nixpkgs
is full of PRs that are already done, but simply lacks a committer to merge
them. And given the enormous size of nixpkgs, we simply don't have enough active
committers.

This is one of the hardest problems to solve, and also one that is hard to solve
in any open source project. On one hand, delegation of responsibility is an
unavoidable part of distributing the maintenance burden, it is simply
unavodiable, but on the other hand, it increases the amount of people who can
cause breakage and harm, whether maliciously or simply on accident.

More granular merge rights would be very useful, and we may be able to get far
with just bots and tooling, but we're also gonna have to find ways to create
more committers, and that often may mean spending time on mentoring that could
instead be spend on tackling features, which people often prefer.

Still I think just with tooling we can get really far here.

Review Culture

A final point that could cause rapid improvement is to codify and disseminate a
stronger review culture. Simple things such as being explicit when something
isn't blocking, learning to not submit a PR until you think it's actually ready,
keeping changes small and reviewable, not reviewing the person but the code, all
of these are often massive improvements to a review process.

There is a bunch of literature on the subject[1], and we should absolutely
normalize using the ideas from this, as well as document it as norms for how to
review PRs. By doing so, we make more people aware of what's good practice, and
also we give them something to refer to in case there is a conflict in a review.

[1]: There recently was an excellent talk on the subject,
https://youtu.be/IvPMsdTS5wg, outlining a lot of what can go wrong in a review,
both from the reviewer's and author's perspective.

@proofconstruction
Copy link
Contributor

The current state of open Issues and PRs in Nixpkgs is frustrating, and I see the occasional self-merge as an expression of this frustration. This issue is perhaps the best example of why we simply need more resources to throw at these kinds of problems. Maintenance of any single package is a serious task - moreso when the dependency tree is deep or complicated, as we often see - and Nixpkgs is the largest repository by a large margin, so adequately dealing with this necessarily requires more from us than from other projects.

As indicated by my answers to other questions, a major focus for me is finding ways to pay maintainers for their work. One way I'd aim to do this is by forming relationships in which commercial organizations could pay the Foundation to support some packages they care about, with the Foundation compensating the relevant maintainers accordingly.

I believe that with enough planning and effort, we can eventually achieve "inbox zero" for open PRs, and we can do the same for the open Issues too.

@NixOS NixOS locked as resolved and limited conversation to collaborators Oct 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

8 participants