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

Optionally request "publisher" in modules #50

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

phaer
Copy link
Member

@phaer phaer commented Dec 6, 2024

Closes #23

@phaer phaer requested a review from zimbatm December 6, 2024 14:18
@phaer phaer marked this pull request as draft December 6, 2024 14:21
@phaer phaer marked this pull request as ready for review December 6, 2024 14:33
lib/default.nix Show resolved Hide resolved
let
module' = if builtins.isString module || builtins.isPath module then import module else module;
wantsPublisher =
builtins.isFunction module' && builtins.hasAttr "publisher" (builtins.functionArgs module');
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of applying the same logic for all the specialArgs? flake, inputs, perSystem. As a module author, I don't think I ever wanted to get access to inputs that are not part of the current flake.

Copy link
Member Author

@phaer phaer Dec 7, 2024

Choose a reason for hiding this comment

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

Not entirely sure, I understand this proposal correctly:

  • If you mean changing the wrapping functions arguments to
    flake, inputs, perSystem instead of publisher. Could do that but the current design was meant to a) keep the api surface small and clearly named and b) still allow people to use the module args flake, inputs, perSystem to refer to the current/consuming flake (without shadowing/need for aliases)

  • If you mean not as an addition, but moving those args to the wrapping function all together instead of module arguments: That would make it difficult to access i.e. packages from the current/consum flake, no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

flake modules don't capture the flake attributes
2 participants