Skip to content

Conversation

@DavidMazarro
Copy link
Member

Closes #238.

@DavidMazarro DavidMazarro self-assigned this Nov 12, 2024
@DavidMazarro DavidMazarro changed the title Addition of an HLint pre-commit hook Migrate of Nix flake to devenv and addition of an HLint pre-commit hook Jan 13, 2025
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 <CristhianMotoche@users.noreply.github.com>
@CristhianMotoche CristhianMotoche requested review from Copilot, oscar-izval and sestrella and removed request for Copilot March 14, 2025 21:46
@CristhianMotoche CristhianMotoche marked this pull request as ready for review March 14, 2025 21:47

# https://devenv.sh/reference/options/
packages = with pkgs; [
pre-commit
Copy link
Contributor

@oscar-izval oscar-izval Mar 17, 2025

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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
Copy link
Member

Choose a reason for hiding this comment

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

@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";
Copy link
Member

Choose a reason for hiding this comment

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

@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.

@DavidMazarro
Copy link
Member Author

Closing in favor of #251. I will be adding the HLint pre-commit hook on a separate PR.

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.

Include hlint as part of the pre-commit and CI process

6 participants