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

lib: Add lib.maintainer-groups #72125

Merged
merged 2 commits into from Mar 10, 2020
Merged

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Oct 27, 2019

There are many people interested in being responsible for core desktop packages but adding themselves to each one of them is a chore. This PR adds an indirection simplifying this use case.

@jtojnar
Copy link
Contributor Author

jtojnar commented Oct 27, 2019

I do not like having to do .maintainers but I still want to have attrsets so we do not have to repeat #36119

@worldofpeace
Copy link
Contributor

Makes sense, I wouldn't have to have a pkgs.pantheon.maintainers or pkgs.gnome3.maintainers anymore (that doesn't make sense at the pkgs level anyway).
I was thinking of adding myself to glib and gtk3 today but this idea returned to me and thought "I'd rather take the time to have this". Guess our idea was floating around again 😄

@jtojnar
Copy link
Contributor Author

jtojnar commented Oct 27, 2019

Yeah, colord-gtk is package without meta.maintainers but I did not want to add myself directly.

@worldofpeace
Copy link
Contributor

So just an idea dump, I think I might've talked about having something where this could map to NixOS Teams so when there's official "working groups" ofborg could request their review. Autolabeing issues for the groups would be nice also, and it would help with people figuring out who's the best person/s to reach on most things. But that's probably an idea that would needs the organizational effort of an RFC.

@jonringer
Copy link
Contributor

I like the idea of having the maintainer-groups being centralized to a single location. Hard to know which packages have maintainers like mesa, gtk, etc

@jtojnar jtojnar marked this pull request as ready for review October 31, 2019 18:38
@aanderse
Copy link
Member

aanderse commented Nov 1, 2019

This is much healthier for nixpkgs core/important packages and will make keeping things up to date so much easier. I love it!

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 1, 2019

Maybe renaming it to lib.teams.freedesktop.members would be little bit nicer to type.

Eventually, we would like to be able to just add a group directly to the meta.maintainers list but that would require changes to OfBorg and Hydra. (Though it should not be a difficult change, just merging and deduplicatiom.) We can leave that for future PRs when this proves useful.

One disadvantage of the current implementation is that it is not possible to find out all packages maintained by a group. That would be useful for maintainers/scripts/update.nix. Maybe we should go directly to phase two as described above.

Also add a freedesktop maintainer group as an example.
@worldofpeace
Copy link
Contributor

@jtojnar So the idea here is that teams are special groups of maintainers that have shared responsibility, ownership, and accountability. They've like maintainers but they operate more collectively, and there's a usually an ongoing internal discourse and larger infrastructures to be accountable for.

I don't think this makes teams a different meta attribute, just a different type of maintainer.
And maintainers list should should be taught groups as you said.

I do think this could be useful right away, it's just when ofborg requests reviews it'll be as individuals.
But people can still read the source description of teams. Phase two as described is a good idea.

@worldofpeace
Copy link
Contributor

Though it is a fundamental thing, should we land more gracefully? It's more difficult to organize changes like this that have many pieces. So if something is committed first it'll be easier down the line.

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 22, 2019

Here is an example how the first-class teams could be handled by ofborg: NixOS/ofborg#421

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 22, 2019

Though it is a fundamental thing, should we land more gracefully? It's more difficult to organize changes like this that have many pieces. So if something is committed first it'll be easier down the line.

On the other hand, not having first-class teams at first would mean having to modify every instance of legacy use. And it could possibly hinder adoption because of people waiting for a better implementation.

@worldofpeace worldofpeace mentioned this pull request Dec 11, 2019
10 tasks
Copy link
Member

@grahamc grahamc left a comment

Choose a reason for hiding this comment

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

I'd like to merge this. I know of a company which would like to have a maintainer group for themselves to attach to packages they as an entity maintain. I added a couple of new fileds, type and membership, because a business-maintained group should probably not casually get more users added to it.

What do y'all think?

@ofborg eval

where

- `members` is the list of maintainers belonging to the group,
- `scope` describes the scope of the group.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `scope` describes the scope of the group.
- `scope` describes the scope of the group,
- `type` "interest-group" or "business",
- `membership` "open" or "closed"; to join a closed group, existing members must approve.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@grahamc
Copy link
Member

grahamc commented Mar 9, 2020

@ofborg eval

@jonringer
Copy link
Contributor

I'm just a little concerned about the following scenario:

meta = with lib; {
  ...
  maintainers = with maintainers; with teams; [ jonringer grahamc ] ++ freedesktop.members;
  ...

hard to know where the attr's are coming from, I guess it could be changed to:

meta = with lib; {
  ...
  maintainers = with maintainers; [ jonringer grahamc ] ++ teams.freedesktop.members;
  ...

similar to the PR, and that's fairly acceptable.

Knowing what attr's are being introduce by a with clause has been a consistent user-pain for nix, even outside of this PR. So I won't hold it against it.

@jtojnar jtojnar merged commit 8515b70 into NixOS:master Mar 10, 2020
@jtojnar jtojnar deleted the maintainer-groups branch March 10, 2020 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants