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

Nix: remove flake-utils + add overlay #55

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

toastal
Copy link
Contributor

@toastal toastal commented Jan 2, 2025

You knew this day was coming 😛

As mentioned elsewhere, there isn’t much point in having a dependency just for a for loop. This will remove the dependency but also expose i686-linux which is supported by tree-sitter but is not in flake-utils’s defaultSystems (which no one seems to check what the defaults are).

Additionally, Nix was missing from the EditorConfig & might as well expose the tree-sitter-nickel as an overlay that automatically appends it into tree-sitter-grammars for convenience (such as for upstream Topiary). All of this & is still a net-negative lines of code diff.

1

Footnotes

  1. Please consider giving up MS GitHub or offering a non-proprietary, non-US-corporate-controlled mirror for this free software project. I wish to delete this Microsoft account in the future, but I need more projects like this to support alternative methods to send patches & contribute.

Copy link
Contributor

@yannham yannham left a comment

Choose a reason for hiding this comment

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

I did know 😛 I think exposing an overlay is nice and makes for an additional argument to not use flake-utils, since they don't fit the system nomenclature. LGTM, thanks!

@yannham
Copy link
Contributor

yannham commented Jan 6, 2025

Checked manually that nix flake check, nix build and nix develop works properly, as our workflow doesn't seem to fire on forks (although it should...)

@yannham yannham merged commit 25464b3 into nickel-lang:main Jan 6, 2025
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