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

[pre-RFC] maintainer-list: add raitobezarius supportedArchitecture #251008

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions maintainers/maintainer-list.nix
Original file line number Diff line number Diff line change
Expand Up @@ -14099,6 +14099,8 @@
github = "RaitoBezarius";
githubId = 314564;
name = "Ryan Lahfa";
supportedArchitectures = [ "x86_64-linux" "riscv64-linux" ];
limitedSupportedArchitectures = [ "aarch64-linux" "x86_64-darwin" "aarch64-darwin" ];
Comment on lines +14102 to +14103
Copy link
Member

Choose a reason for hiding this comment

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

2 suggestions i have

  • architectures -> platforms to be more consistent with meta.platforms, also architecture is sometimes referred to only the x86_64 and riscv64 part, but not the kernel
  • limitedSupportArchitectures feels a bit arbitrary and verbose, having a list of list allows users to have more than 2 tiers of support
Suggested change
supportedArchitectures = [ "x86_64-linux" "riscv64-linux" ];
limitedSupportedArchitectures = [ "aarch64-linux" "x86_64-darwin" "aarch64-darwin" ];
supportedPlatforms = [
[ "x86_64-linux" "riscv64-linux" ]
[ "aarch64-linux" "x86_64-darwin" "aarch64-darwin" ]
];

Copy link
Member Author

Choose a reason for hiding this comment

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

limitedSupportedArchitectures comes from nixos/release-*.nix BTW.

Copy link
Member

Choose a reason for hiding this comment

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

Platforms follow the tier system. We can replay them here somehow.

Copy link
Member

@infinisil infinisil Aug 23, 2023

Choose a reason for hiding this comment

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

That doubly-nested listing is rather hard to understand, and I don't think we need to use the same tiering from NixOS/rfcs#46. How about

primaryPlatforms = [ ... ];
secondaryPlatforms = [ ... ];

Or maybe this would be better, more directly correlating to the expectations:

# Using these regularly
regularPlatforms = [ ... ];
# I have these available if necessary
availablePlatforms = [ ... ];

Copy link
Member

Choose a reason for hiding this comment

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

Maybe even primary/available. I do use a bit of Nix-managed stuff on aarch64-linux daily… but really not enough to have primary-platform competence there.

Copy link
Member

Choose a reason for hiding this comment

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

How about primary/supported? (Maybe with a check that the former is a sub set of the latter (Speaking of which, that maintainers list doesn't seem to be type/sanity checked anywhere?))

Copy link
Member

Choose a reason for hiding this comment

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

I like primary/supported and primary/available, though I wouldn't repeat the primary ones in supported/available.

};
rakesh4g = {
email = "[email protected]";
Expand Down