Migrate of Nix flake to devenv and addition of an HLint pre-commit hook #249
Conversation
DavidMazarro
changed the title
Run cabal update before test Update CI configuration Remove devenv shell Cache cabal dependencies Comment Docker workflow Set locales Add env var Debug locale test passing env var in the same command Revert locale changes Update runner chore: Attempt to include disable unicode diagnostics chore: Document variables set on flake.nix Co-authored-by: Cristhian Motoche <[email protected]>
CristhianMotoche
requested review from
oscar-izval,
sestrella and
Copilot
and removed request for
Copilot
|
|
||
| # https://devenv.sh/reference/options/ | ||
| packages = with pkgs; [ | ||
| pre-commit |
There was a problem hiding this comment.
I think you can simplify the setup a bit by defining the pre-commit hooks with the implementation that devenv exposes by default. You can find more details here. That way you don't have to track the config file in git and you can get rid of the entershell command
There was a problem hiding this comment.
Using pre-commit instead of devenv's own Git hook mechanism was intentional: that way, even if someone working on hapistrano decides not to use devenv, they can still make use of pre-commit hooks. Which reminds me that I should add that a note about setting up pre-commit to the regular development instructions, I'll do that.
| devenv, | ||
| systems, | ||
| ... | ||
| } @ inputs: let |
There was a problem hiding this comment.
@DavidMazarro I'm wondering if the complexity of this file comes from the fact that we are using it for two different purposes. On one hand, it appears that we want a portable development environment, and on the other, we want to provide a Nix flake version of this project that can be properly referenced in other Nix projects. If that's the case, I'd recommend creating a standalone devenv.nix file for local development and leaving the current flake.nix with only the packages declaration; this should simplify some maintenance tasks as devenv is a higher-level abstraction over Nix flakes, making it more approachable for new comers.
| }; | ||
| }; | ||
| packages = { | ||
| default = flake.packages."hapistrano:exe:hap"; |
There was a problem hiding this comment.
@DavidMazarro If we intend to remove the 'default' package, we should consider doing so as a breaking change, as other Nix flakes may already be referencing this project.
|
Closing in favor of #251. I will be adding the HLint pre-commit hook on a separate PR. |
Closes #238.