search

Infinidoge / nix-minecraft

An attempt to better support Minecraft-related content for the Nix ecosystem
MIT License
246 stars 26 forks source link

refactor(flake): replace digga with flake-utils #4

Closed Infinidoge closed 2 years ago

Infinidoge commented 2 years ago

This replaces digga, which is bulky and contains a lot of unnecessary dependencies, with flake-utils, which is a single dependency that maintains the normal flake construction process. (As opposed to a wrapper like mkFlake.)

Resolves #3

Additionally, this replaces the overlay's hardcoded x86-64_linux with a reference to the host's system, making the overlay cross-platform.

Infinidoge commented 2 years ago

Oh making a pull request pulls in the conversation from commits, huh. Today I Learned.

soupglasses commented 2 years ago

A recommendation I would give would be to use the name flake-utils instead of fu on the flake-utils input, this makes it more apparent where the input is from, and importers can then write inputs.nix-minecraft.inputs.flake-utils.follows = "flake-utils" instead of inputs.nix-minecraft.inputs.fu.follows = "flake-utils" if desired.

soupglasses commented 2 years ago

Tested this PR on my own repo https://github.com/imsofi/phenix/commit/9a90b317357d3f50a3240475166e439f916d7407. It works without needing to change any implementations from my side. Do note that I do not load the overlay or the nixosModule, so these i did not check.

Great work! 🎊

Infinidoge commented 2 years ago

A recommendation I would give would be to use the name flake-utils instead of fu on the flake-utils input, this makes it more apparent where the input is from, and importers can then write inputs.nix-minecraft.inputs.flake-utils.follows = "flake-utils" instead of inputs.nix-minecraft.inputs.fu.follows = "flake-utils" if desired.

Went ahead and renamed it. Just going to test on my own setup, since I make use of both the overlay and module, then rebase on top of master.

Infinidoge commented 2 years ago

prev.system didn't work, however I believe I figured out a solution that I think is cleaner overall.

Infinidoge commented 2 years ago

I need to fix an error where for some reason it isn't finding mapAttrs in prev.lib when using the overlay, but it appears to work.

Infinidoge commented 2 years ago

It appears to be working just fine now, could you test on your end to double check?

soupglasses commented 2 years ago

I see no problems on my end, but i only use the packages currently, so my testing is limited.

Infinidoge commented 2 years ago

Alright, cool. Seems to work fine on my end, which uses the services, so I'll go ahead and rebase and merge.