-
Notifications
You must be signed in to change notification settings - Fork 20
flake/hjemModule: expose modulesPath, stop mass importing by default #70
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
base: main
Are you sure you want to change the base?
Conversation
Forgot to fix tests, gotta quickly fix that. |
I'm not entirely sure that this is a good idea. The technicall implementation appears sound, but we have to remember that Hjem-Rum is designed to bridge the gap between Hjem (the performant option) and Home-Manager (the non-performant option). I wonder if we could do something like |
0994abf
to
ef3eb74
Compare
One option is having Edit: Improved possible hjemModule names. |
49f28e7
to
96a32c7
Compare
Updated with an improved new user experience. |
96a32c7
to
923598b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, this review is more of a suggestion than anything. Feel free to dismiss it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the implementation! My big question is about whether we can handle dependencies internally - everything else is nitpicks.Pardon any line comments that are actually referring to an entire paragraph - I'm not very good at Github review.
> Hjem continues to be developed, Hjem Rum will be worked on as we build modules | ||
> and functionality out to support average users. | ||
> ready to fully replace Home Manager in the average user's configuration, but | ||
> if you truly want to, an option could be to use both in conjunction. Either |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot of unrelated changes in this commit, that should be split into their own commit.
Some users may wish to use hjr but be discouraged by lengthening eval times. For that reason, we have added an output that links to the module collection, extensively modified README.md, and stopped importing the entire module collection by default. Users who still want to import the entire module collection automatically can use `lib.filesystem.listFilesRecursive`, as explained in the readme.
923598b
to
1514036
Compare
Would it be possible to individually import specific modules? That way you're only importing precisely what's needed rather than some vague "bare" or a maximal "complete" |
That's exactly what the bare module is for. The bare module just extends Hjem for our modules to function properly, you then import specific modules as desired. This PR is currently in major limbo for various reasons though, I'm not sure if it will see the light of day any time soon. It'll require major work to make mergeable and still has some annoyances that leads to nobody really being happy with it. As it is, even if we got it merged, it would add more chores for contributors and maintainers to keep track of. Ideally, I would want a solution to solve the issue that works seamlessly for those who want it while also not bloating the maintenance burden or conflicting with the intended audience's experience, namely those who don't mind the eval bloat and don't want to learn to write their own modules. For you and anyone else interested in this PR, I would suggest you just figure out your programs yourself and learn how to write your own modules for your own config. Or if you really want to make this work, go ahead and try to write your own PR for it and see if it meets approval. I would truly be impressed if someone actually managed it, even just a prototype would be interesting to see. |
Some users may wish to use HJR but be discouraged by lengthening eval times. For that reason, we have added an output that links to the module collection, extensively modified README.md, and stopped importing the entire module collection by default.
Users who still want to import the entire module collection automatically can use
lib.filesystem.listFilesRecursive
, as explained in the readme.While I am marking this as ready for review since it does in fact work in my config, I am not entirely set on this approach and would love some feedback.
Closes #65.