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

flake: defragment lib+wrappers, also cleanup tests #2104

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MattSturgeon
Copy link
Member

  • lib: include standalone wrapper in top-level lib
  • tests: use callPackage pattern for flake checks

Copy link
Contributor

@khaneliman khaneliman left a comment

Choose a reason for hiding this comment

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

Overall, it feels cleaner and easier to follow the dependency chain of setting up checks. I like not having all the boilerplate for one liners in a few files and searching for the definition.

Looks like something isn't setup right though according to buildbot.


# TODO: Consider renaming these?
makeNixvimWithModule = import ../wrappers/standalone.nix pkgs flake;
makeNixvim = module: self.makeNixvimWithModule { inherit module; };
Copy link
Contributor

Choose a reason for hiding this comment

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

what were you thinking for these names?

Copy link
Member Author

Choose a reason for hiding this comment

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

mkNixvim and mkNixvimWith

  • it seems we usually prefer mk over make, although it's not consistent
  • the With suffix is conventionally used in nixpkgs for a more powerful function variant that takes structured attr args
  • WithModule is misleading, because both functions take a module

Copy link
Contributor

Choose a reason for hiding this comment

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

* `WithModule` is misleading, because both functions take a module

This is what was throwing me when looking at their definitions, as well...

Copy link
Member Author

@MattSturgeon MattSturgeon Sep 7, 2024

Choose a reason for hiding this comment

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

While I'd like to rename these, I'm hesitant to do so yet as there's other changes I'd like to make and I think we should minimize user-facing refactoring where possible.

In particular, I'd like to get to a point where our lib (including the standalone wrapper) doesn't depend on pkgs or system. This matches the design of other similar flakes like nixos and home-manager, whose equivalent function (e.g. nixpkgs.lib.nixosSystem) is made available without being in the "perSystem" part of the flake outputs.

Achieving this goal may be fairly involved though and require a few breaking changes and ugly compromises, including re-imagining the standalone wrapper and it's function signature. That'd be the optimal time to rename it, I think.

EDIT: #2186

- Defragment lib-related flake-modules
- Expose the standalone-wrapper functions in the `lib`
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.

2 participants