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

Create .pre-commit-hooks.yaml #238

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

Jayman2000
Copy link
Contributor

This will allow people to enforce the style of .nix files in their own repos via the pre-commit tool. This change is related to #221, but it doesn’t actually implement the idea proposed in that issue.

Copy link

github-actions bot commented Aug 23, 2024

Nixpkgs diff

Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Aug 23, 2024
Making this change was unexpectedly annoying (yak saving 🙁). All I
wanted to do was add a flake.nix file to another private Git repo.
Before I could add the flake.nix file, I had to open a pull request for
nixfmt [1] so that I could create four different commits in this repo so
that I could make sure that Nix files get styled properly so that I can
START creating the flake.nix file in that other repo. Plus, there will
probably be more problems that I encounter while creating the flake.nix
file, and I’ll end up having to make more changes to this repo.

In other words, this is just the beginning of the yak shaving.

[1]: <NixOS/nixfmt#238>
@infinisil
Copy link
Member

How does this work? It would be great if you could add this to the README :)

This will allow people to enforce the style of .nix files in their own
repos via the pre-commit tool. This change is related to #221, but
it doesn’t actually implement the idea proposed in that issue.

When writing the instructions for how to use the nixfmt pre-commit hook,
I tried to make it clear that there’s a difference between the
pre-commit tool and Git pre-commit hooks in general. I tried to make the
difference between those two things clear for two reasons:

1. Both of those things are called pre-commit which makes it easy for
people to mix them up [1].

2. Depending on how #221 shakes out, the nixfmt project might end up
adding another alternative mechanism for using nixfmt as part of a Git
pre-commit hook script. I’m preemptively using language that makes it
easy to differentiate between those two different mechanisms just in
case there ends up being two different mechanisms in the future.

[1]: <DescentDevelopers/Descent3#450 (comment)>
@Jayman2000
Copy link
Contributor Author

I just pushed a new version that adds instructions to the README. If you want to test out those instructions, then you’ll need to make some tweaks once you get to step 5 in order to account for the fact that this PR hasn’t been merged yet. You’ll need to replace the https://github.com/NixOS/nixfmt URL with https://github.com/Jayman2000/nixfmt-pr, and you’ll need to use 6d57d1b as the version.

Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

This is looking great to me!

@infinisil infinisil merged commit 8c4da7b into NixOS:master Aug 27, 2024
2 checks passed
@Jayman2000 Jayman2000 deleted the pre-commit.com-hook branch September 5, 2024 23:04
Jayman2000 added a commit to Jayman2000/jasons-pre-commit-hooks that referenced this pull request Sep 6, 2024
Now that this PR [1] has been merged into nixfmt, we can use the
upstream nixfmt repo.

[1]: <NixOS/nixfmt#238>
@Freed-Wu
Copy link

nixfmt’s integration with the pre-commit tool is relatively new. At the moment, none of the stable releases of nixfmt can be used with the pre-commit tool. You’ll have to use an unstable version for the time being.

When to release a new version? Just increase subrevision by one is OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants