Skip to content

Structured maintainers#31739

Closed
moretea wants to merge 3 commits intoNixOS:masterfrom
moretea:structured_maintainers
Closed

Structured maintainers#31739
moretea wants to merge 3 commits intoNixOS:masterfrom
moretea:structured_maintainers

Conversation

@moretea
Copy link
Contributor

@moretea moretea commented Nov 16, 2017

Motivation for this change

Enables having proper metadata about maintainers, which is useful for NixOS/rfcs#19 (maintainers file) / NixOS/rfcs#21 (PR's on Hydra).

Example
nix-instantiate --strict --eval . -A pkgs.keen4.meta.maintainers
[ { GitHub = "edolstra"; IRC = "niksnut"; email = "eelco.dolstra@logicblox.com"; name = "Eelco Dolstra"; } ]
Things done

Verified that not everything is completely broken by running:

nix-build nixos/tests/boot.nix 
nix-build nixos/tests/firefox.nix
Why the manipulation of the data structure at the bottom of the file?

If addGitHub would not be applied to the attrset, then keys of the attrset cannot be observed when accessing them via pkgs.my_pkg.meta.maintainers.

Remaining questions
  • Is setting IRC to null if not defined desired behavior?

@moretea moretea requested review from edolstra and nbp as code owners November 16, 2017 12:09
@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Nov 16, 2017
lib/attrsets.nix Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed? ... considered harmful.

@edolstra
Copy link
Member

This seems overly complicated to me. Why do we need that all that funky _structuredMaintainers and ... business? I would just change lib/maintainers.nix to return a attrset of attrsets.

@moretea
Copy link
Contributor Author

moretea commented Nov 16, 2017

I would very much prefer to only have that!

My reasoning for implementing a flag is based on:

  1. This structured data is very useful for the RFCs 19&21 to ping the correct maintainers.
  2. I assumed that hydra depends on pkgs.my_pkg.meta.maintainers to be a list of email addresses.

Having a flag will allows us to get to use this data before support is added to Hydra.

@moretea moretea force-pushed the structured_maintainers branch from 1819fe9 to 9520698 Compare November 16, 2017 19:31
@moretea
Copy link
Contributor Author

moretea commented Nov 16, 2017

We discussed this on IRC, and we'll just move to the new format, after which @edolstra will fix Hydra.

@orivej orivej mentioned this pull request Nov 26, 2017
8 tasks
@FRidh
Copy link
Member

FRidh commented Feb 11, 2018

I would like to take this further (see #34842 (comment)). @moretea did you use a script to generate the attr sets?

@7c6f434c
Copy link
Member

I think the feature has been implemented somehow, so any PRs about maintainer list structure should be discussed relative to what we have now.

@7c6f434c 7c6f434c closed this Jun 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants