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

build: Add treefmt-nix #76

Closed
wants to merge 1 commit into from
Closed

build: Add treefmt-nix #76

wants to merge 1 commit into from

Conversation

KP64
Copy link
Contributor

@KP64 KP64 commented Jan 29, 2025

This PR removes pre-commit checks and delegates those to the CI when the treefmt checks are run by check-flake.yml.

Numtide dev shells have been removed too as they are replaced by just using nix fmt, reducing the flake size.
Standard dev shells are taking their place instead with the mdbook package, which I've found was missing when I tried to serve the documentation locally.

The formatter is left to be Alejandra. However, it would be beneficial to change to nixfmt-rfc-style (nixfmt in treefmt-nix) in order to guarantee the same styling as in Nixpkgs.

@jaredmontoya
Copy link
Contributor

jaredmontoya commented Feb 4, 2025

@KP64 did you see what #46 was originally?
It's main purpose was to reduce the flake size and replace pre-commit-hooks with treefmt-nix (it didn't happen)

Since then I maintain a branch of nix-topology that has only these inputs just so that my NixOS configuration can have less dependencies:

    nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
    systems.url = "github:nix-systems/default";

    treefmt-nix = {
      url = "github:numtide/treefmt-nix";
      inputs.nixpkgs.follows = "nixpkgs";
    };

Also my fork removes fail2ban support because fail2ban icon is ugly. But fail2ban is important and is running on all my servers so there are multiple ugly icons everywhere. please find a better icon for it and submit it to upstream nix-topology. then I will remove the revert commit next time I rebase nix-topology

@KP64
Copy link
Contributor Author

KP64 commented Feb 6, 2025

Ah, you are right. Didn't see your PR.
Based on your discussion I don't expect this PR to land then.
Will close. ;)

P.S.:
I endorse your statement about fail2ban and similar services cluttering the topology.
This definitely needs a different solution, like the one you proposed 👍

please find a better icon for it and submit it to upstream nix-topology.

I agree it does look bad. But that's not a priority to be honest. If someone finds a better Icon and actually cares about it then they would submit it, or disable it altogether. Since it is the official Icon though I'm pretty much indifferent though. :)

@KP64 KP64 closed this Feb 6, 2025
@KP64 KP64 deleted the build/treefmt branch February 6, 2025 13:22
@oddlama
Copy link
Owner

oddlama commented Feb 6, 2025

Ah, you are right. Didn't see your PR. Based on your discussion I don't expect this PR to land then. Will close. ;)

I still intend to switch to flake-parts at some point and introduce nix-treefmt afterwards, which is what I already did in all many of my other projects. I just also want to keep pre-commit-hooks which calls treefmt to have a basic level of checks before making a commit.

I see that this is far from perfect from a dependency minimization point of view, but as long as we don't have a way to separate dev-only and "runtime" dependencies in flakes, we will always have dependencies pulled in which are unnecessary for users. But when developing a project you do want to have a devshell and some other things, so I don't see how we can work around this without arbitrarily limiting what we use.

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.

3 participants