Add replace xyzBuilder with unified 'outputsBuilder' #59
Conversation
| (optionalAttrs (args ? ${builder}) | ||
| { ${output} = args.${builder} pkgs; }); | ||
| (optionalAttrs (systemOutputs ? ${output}) | ||
| { ${output} = systemOutputs.${output}; }); | ||
|
|
There was a problem hiding this comment.
I think this would be better done with a mapAttrs. That way its future proofed if more system-spaced outputs ever get added.
There was a problem hiding this comment.
also code is a bit simpler, mkOutput is no longer necessary
There was a problem hiding this comment.
Yes, I agree mapAttrs would be better option but we are dealing with multiple restrictions:
nix flake showshould not error - we cant output empty attrsets- outputs must be merged - if we just go with mapAttrs while keeping in mind the first point - not all attrs will get merged.
And yeah, I guess I could go over the names and just generate attrset out of that. Probably will change that but I really should go and get some sleep now :D
There was a problem hiding this comment.
sounds good, I think it could probably be used in the future once we remove the deprecated arguments. For now this works well.
There was a problem hiding this comment.
Not going to do the attrs from list thingy. At least for time being - could not find a solution that I'd be happy with. Its other solutions are more complex and use around the same amount of lines
|
Also, not sure what happened to GitHub actions - but it does not show output for some reason 🤔 Anyways, there are two checks - one uses What needs to be tackled before merge:
If you guys wish to work on it - feel free to just push changes to this branch. But I'm pretty sure that ill find some time tomorrow to fix this up On a totally unrelated note - @blaggacao thank you for talking me into adding devshell <3 It is so dam convenient |
|
Nice! |
No description provided.