Conversation
Sets allow to instantiate pkgs with different overlays. This first change is a pure refactoring and should not have functional changes. The 'base' set is equivalent to the previous single 'pkgs' instance.
|
katexochen
force-pushed
the
p/contrast-builder-v2
branch
4 times, most recently
from
b464de2 to
6df49ae
Compare
charludo
left a comment
charludo
left a comment
There was a problem hiding this comment.
This mostly looks good to me. Not gonna pretend I 100% understand 17f577e though 😅
Would you be OK with aliasing the base set to the top-level output? I understand why it makes sense to have .#base.*, esp. in the justfile to easily toggle sets, but having to run e.g. nix run .#base.scripts.generate or nix build .#base.contrast.docs instead of just nix run .#scripts.generate or nix build .#contrast.docs seems counter-intuitive when those packages do not depend on the kata image at all, and are also already scoped either under scripts or contrast.
I thought about this, too, but I'm unsure about it. The way it currently is it is verbose and might be a bit annoying to type, but moving base top-level would bear the risk of devs writing things in a way that is not "set-aware", thus increasing the mental load during development/review. Another point is that while it might be easier to use, it complicates the nix structure hand makes it even harder to work with. Your choice. :) |
katexochen
force-pushed
the
p/contrast-builder-v2
branch
from
6df49ae to
5b52458
Compare
charludo
left a comment
charludo
left a comment
There was a problem hiding this comment.
I thought about this, too, but I'm unsure about it. The way it currently is it is verbose and might be a bit annoying to type, but moving base top-level would bear the risk of devs writing things in a way that is not "set-aware", thus increasing the mental load during development/review. Another point is that while it might be easier to use, it complicates the nix structure hand makes it even harder to work with. Your choice. :)
Yeah after what we've discussed, I get your reasoning, and as I said, this wasn't really a technically reasoned complaint. So, let's keep it as is.
I want to play around with the scripts + // vs overrideScopes issue we just talked about, but I'll do that regardless of this PR, so that should also not block this, since it should not currently be an issue afaik.
This is required for overrides to work correctly. Previously, overriding a package in one scope (e.g. 'kata') wouldn't propagate properly into another scope (e.g. 'contrast') that depends on that package from the first scope.
katexochen
force-pushed
the
p/contrast-builder-v2
branch
from
5b52458 to
f408f7a
Compare
2 participants
No description provided.