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.

Sign up for GitHub

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

Jump to bottom

maintainers: document expectations #250344

Merged
merged 11 commits into from
Sep 1, 2023
Merged

maintainers: document expectations #250344

Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
34c2c3e
maintainers: document expectations
zimbatm Aug 20, 2023
a888774
change the wording to be a bit more lax on losing maintainer status
zimbatm Aug 20, 2023
10e3f2c
clarify how removal happens
zimbatm Aug 23, 2023
c67b737
expand the reasoning a bit more
zimbatm Aug 23, 2023
9afac4c
Update maintainers/README.md
zimbatm Aug 23, 2023
405ea93
Update maintainers/README.md
zimbatm Aug 23, 2023
9025fb6
Update maintainers/README.md
zimbatm Aug 23, 2023
e7d700b
Update maintainers/README.md
zimbatm Aug 23, 2023
0f92e2c
Update maintainers/README.md
zimbatm Aug 23, 2023
4f78788
Update maintainers/README.md
zimbatm Aug 23, 2023
14d76e5
Update maintainers/README.md
zimbatm Aug 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions maintainers/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,49 @@
# Nixpkgs Maintainers

The *Nixpkgs maintainers* are people who have assigned themselves to
maintain specific individual packages. We encourage people who care
about a package to assign themselves as a maintainer. When a pull
request is made against a package, OfBorg will notify the appropriate
maintainer(s).
Unlike other packaging ecosystems, the maintainer doesn't have exclusive
control over the packages and modules they maintain. This more fluid approach
is one reason why we scale to so many packages.


## Definition and role of the maintainer

The main responsibility of a maintainer is to keep the packages they maintain
in a functioning state, and keep up with updates. In order to do that, they
are empowered to make decisions over the packages they maintain.

By default, we expect committers to wait at least a week before merging
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a mention for packages that are not affected by the "one week" rule, that is: "code owned" packages which have a large reverse dependency tree, e.g. systemd, LLVM, etc.

I don't think it's easy to put an amount of time, I'd state it as:

Committers should reach out to the active code owners and discuss the timing for a potential merge, in the event where no active code owner can be reached in a reasonable amount of time (i.e. taking into account potential holidays), merge can be performed nevertheless with a provided rationale.

The idea is to:

(1) ensure that no stray committer merges a systemd change while not being in touch with active code owner of e.g. systemd which may have extra context on ongoing changes (internal information, etc.)

(2) ensure that abandoned "important dependencies" can be slowly picked up by new committers who will garden them and find a new maintenance story for them hopefully, e.g. LLVM

Tiny version bumps and security updates could not be totally exception here, I don't have a strong opinion on that, I know that systemd security updates can lead to breakage, we had e2fs tooling's tiny version bumps breaking bootloaders (something something XFS), etc. It's a very hard problem in general.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny version bumps and security updates could not be totally exception here, I don't have a strong opinion on that, I know that systemd security updates can lead to breakage, we had e2fs tooling's tiny version bumps breaking bootloaders (something something XFS), etc. It's a very hard problem in general.

Would it make sense to make an exception for security updates / fixes earlier if e.g. the security team is involved as reviewer?

Generally I think that case of e.g. important bugfixes (e.g. $bump fixes data loss on runtime) or security fixes it should be possible for a committer to decide on a case-by-case basis what's best. @RaitoBezarius has brought some great examples on why I doubt that it's possible to find a general rule for this.

Also, such a ruling would exclude all those cases where the sole motivation of a person not maintaining the package in question is e.g. "wants to have the latest version available, immediately".

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk Aug 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with @RaitoBezarius on specifying timing, or rather pushing that responsibility to maintainers themselves. Maybe we could establish a convention where maintainers are supposed to communicate what they can deal with.

That information could also be used to judge if someone is inactive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you look at c67b737 and tell me if that is good enough? I'm keeping the text short so people are more likely to read it. And therefor try to not overload with exceptions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need better annotations on the meta attributes for the maintainers to communicate some of these aspects to the other committers. Specifically, maintainers should list the architectures they are willing to maintain. But that's for another PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting point @ architectures they are willing to maintain.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #251008 for exploring the question, feel free to carry the conversation there.

changes on packages they are not maintaining. This should leave enough time
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking, this does not make an exception for a non-committer-maintainer approved changes. Maybe it would be better writing to say it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you expand on what you mean? I'm not sure to understand

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

«By default, we expect committers to wait at least a week before merging changes on packages they are not maintaining.» — however, we do not currently wait to give all maintainers time to comment, if one non-committer maintainer asks to merge, this is currently supposed to be good enough when the committer also likes the change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also realize we don't describe how conflict resolution happens in case multiple maintainers of the same package disagree.

zimbatm marked this conversation as resolved.
Show resolved Hide resolved
for the maintainers to provide feedback. The only exception would be for tiny
version bumps and security updates that could be merged faster.

In case of conflict, the maintainer takes priority and is allowed to revert
the changes. This can happen for example if the maintainer was on holiday.

### How to become a maintainer

We encourage people who care about a package to assign themselves as a
maintainer. You don't require to have nixpkgs commit access to do so.
zimbatm marked this conversation as resolved.
Show resolved Hide resolved

In order to do so, add yourself to the
[`maintainer-list.nix`](./maintainer-list.nix), and then to the desired
package's `meta.maintainers` list, and send a PR with the changes.
AndersonTorres marked this conversation as resolved.
Show resolved Hide resolved

### How to lose maintainer status

Maintainers that have become inactive on a given package can be removed. This
zimbatm marked this conversation as resolved.
Show resolved Hide resolved
helps us keep an accurate view of what parts of nixpkgs are well maintained or
not.
zimbatm marked this conversation as resolved.
Show resolved Hide resolved

The inactivity measure is currently not strictly enforced. We would typically
look at it if we notice that the author hasn't reacted to package-related
notifications for more than 3 months.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that "no reaction at all" is meant here, correct? I.e. this isn't done on a per-package basis (otherwise one could lose the status if a fellow maintainer is always faster and you don't have anything to add most of the time).

When I first read this paragraph, I wasn't sure, so perhaps this should be rephrased a bit to make this clearer.

Also, we'd to give people who are about to lose their status a heads-up, correct? I guess this should be mentioned here as well :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this is a non-pressing change to a package one is a maintainer of, and I guess «wait a week or a bit more for maintainer reaction» should be treated as «maintainer in question» in this case…

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing a maintainer requires making a PR so that that person would be notified. But I'll add a paragraph for that.

I still think the test should be per package. Some people are active in some areas of nixpkgs, and have abandoned other parts that they don't care about anymore.

Worst case you can react on the removal PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10e3f2c

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, pretty sure I have completely forgotten about some packages I have maintained back when I used them…


The maintainer is welcome to come back at any time.

### Tools for maintainers

When a pull request is made against a package, OfBorg will notify the
appropriate maintainer(s).
Janik-Haag marked this conversation as resolved.
Show resolved Hide resolved

## Reviewing contributions

Expand Down