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

team-list.nix: nixos-modules -> module-system #255905

Merged
merged 3 commits into from
Sep 19, 2023

Conversation

roberth
Copy link
Member

@roberth roberth commented Sep 18, 2023

The module system isn't about NixOS, so for consistent messaging, I'd like to rename this.

The descriptions are conflicted about whether it's the one or the other, so I picked one. I suppose we could have two teams instead?

If some of you would like to be in a team responsible for NixOS' use of the module system, that would be great, but for consistency it should be a separate team.

@alyssais @Ericson2314 @infinisil, what do you think the scope should be? Should we do this, or do we want this team to be about NixOS instead, and de-emphasize the module system part? In that case we could have two teams.

This attribute isn't referenced explicitly anywhere - only generically at release time for the status update pings, so it's valid as a one file change.

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot 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 Sep 18, 2023
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

I think it would be okay for this team to also include NixOS' use of the module system (but definitely not arbitrary NixOS modules).

Comment on lines 648 to 649
scope = "Maintain Nixpkgs module system internals.";
shortName = "Module system / internals";
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 = "Maintain Nixpkgs module system internals.";
shortName = "Module system / internals";
scope = "Maintain the Nixpkgs module system and its use in NixOS. Out of scope are NixOS modules themselves.";
shortName = "Module system";

Copy link
Member Author

Choose a reason for hiding this comment

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

By saying "out of scope are NixOS module themselves" you've basically negated "and its use in NixOS".
Some care for callers is implied by virtue of being a library, so I don't think we need to try to describe that here, in a roundabout way.

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 = "Maintain Nixpkgs module system internals.";
shortName = "Module system / internals";
scope = "Maintain the Nixpkgs module system.";
shortName = "Module system";

Fair enough. I think the "internal" should be removed then though, because we also maintain its surface.

@alyssais
Copy link
Member

@alyssais @Ericson2314 @infinisil, what do you think the scope should be? Should we do this, or do we want this team to be about NixOS instead, and de-emphasize the module system part? In that case we could have two teams.

It's news to me that this team even exists, let alone that I'm on it. While you're here, could you just remove me?

@infinisil
Copy link
Member

It's news to me that this team even exists, let alone that I'm on it.

Same actually, seems like @dasJ added this team in #165978 without an explanation how the members were decided. It seems to be used for the release blockers issues like #224457 though, and I'm fine with being listed in this team for that purpose.

maintainers/team-list.nix Outdated Show resolved Hide resolved
maintainers/team-list.nix Outdated Show resolved Hide resolved
roberth and others added 2 commits September 18, 2023 21:16
The module system isn't about NixOS, so for consistent messaging,
I'd like to rename this.

If some of you would like to be in a team responsible for NixOS'
_use_ of the module system, that would be great, but for consistency
it should be a separate team.

Note that this attribute isn't used anywhere - only generically at
release time for the status update pings.
qyliss confirmed this
Haven't seen ericson2314 do maint work on it, and he'd tell me if he wanted to.

Co-authored-by: Silvan Mosberger <github@infinisil.com>
Co-authored-by: Silvan Mosberger <github@infinisil.com>
@infinisil infinisil merged commit 7362915 into NixOS:master Sep 19, 2023
4 checks passed
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.

3 participants