stylix icon indicating copy to clipboard operation
stylix copied to clipboard

treewide: add linters and formatters

Open trueNAHO opened this issue 1 year ago • 2 comments

About

This is a tracking issue for integrating linters and formatters.

Steps

  • [X] https://github.com/danth/stylix/pull/228
  • [x] https://github.com/danth/stylix/pull/426
  • [ ] treewide: integrate linters and formatters
    • [ ] GitHub Actions
    • [ ] nix develop
    • [ ] nix flake check
    • [ ] nix fmt

Unresolved Questions

  • [ ] What linters and formatters should be integrated?
    • https://github.com/danth/stylix/pull/426#issuecomment-2168168100

trueNAHO avatar Jan 31 '24 18:01 trueNAHO

 treewide: integrate linters and formatters into nix develop

What is the reasoning for this to be integrated into nix develop instead of just using nix fmt?

treewide: integrate linters and formatters into nix flake check

ci: integrate linters and formatters into GitHub Actions

Maybe treefmt-nix would be a good solution? The config could look something like..

_: {
  # Project root
  projectRootFile = "flake.nix";
  programs = {
    nixfmt.enable = true;
    deadnix = {
      enable = true;
      # Can break callPackage if this is set to false
      no-lambda-pattern-names = true;
    };
    statix.enable = true;
  };
}
``

LovingMelody avatar Jul 21 '24 00:07 LovingMelody

 treewide: integrate linters and formatters into nix develop

What is the reasoning for this to be integrated into nix develop instead of just using nix fmt?

nix develop and nix flake simplify CI integration:

  • I acknowledge pre-commit hooks being controversial. Nonetheless, they integrate nicely with nix flake check and nix develop, simplifying GitHub Actions and arguably improving the developer feedback loop.

    -- https://github.com/danth/stylix/pull/426#issuecomment-2168168100

  • Interesting, hooks might make it more convenient for regular contributors. Having it run on CI too ensures nothing is missed.

    -- https://github.com/danth/stylix/pull/426#issuecomment-2168792953

I added nix fmt to the Steps of this tracking issue.

treewide: integrate linters and formatters into nix flake check ci: integrate linters and formatters into GitHub Actions

Maybe treefmt-nix would be a good solution? The config could look something like..

_: {
  # Project root
  projectRootFile = "flake.nix";
  programs = {
    nixfmt.enable = true;
    deadnix = {
      enable = true;
      # Can break callPackage if this is set to false
      no-lambda-pattern-names = true;
    };
    statix.enable = true;
  };
}
``

Choosing the formatter is somewhat controversial. IIRC, nixfmt tries to touch as few lines as possible to reduce merge conflicts in Nixpkgs, which is one of the most active GitHub repositories. However, since Stylix is nowhere near as active as Nixpkgs we can choose a more beautiful formatter, touching more lines.

For example, alejandra is my favourite Nix formatter:

For reference, I use git-hooks.nix to set up the following linter and formatter hooks in my dotfiles repository:

-- https://github.com/danth/stylix/pull/426#issuecomment-2168168100

I would resolve this issue by first refactoring the GitHub Action to run a single nix command, and then replacing that nix command's implementation with a pre-commit hook.

trueNAHO avatar Jul 21 '24 13:07 trueNAHO