Move style checks to pre-commit

[ahans]

This is roughly what I had in mind. It allows you to maintain only .pre-commit-config.yaml for all style checks. The GH action runs it in CI, manually you can just pre-commit run [--all] and don't have to manage tools or anything manually. It's all taken care of by pre-commit.

Just putting this up to show what I was talking about. No need to merge this if you don't like it.

[nachovizzo]

This I'd keep 14, just because is the default version of Ubuntu 22.04, it might break with newer versions

[ahans]

Alright, will revert it then.

[nachovizzo]

How does a failure case look like? Is it clear if the failure is on black or any formatter? Would you mind providing a screenshot or something similar

[nachovizzo]

@ahans sorry I just merged main again into this branch. I keep accidentally pushing the damn button 😂. I still can't believe I can merge stuff into your private repo 😱. Sorry I did it twice 😔

[ahans]

How does a failure case look like? Is it clear if the failure is on black or any formatter? Would you mind providing a screenshot or something similar

Here's an example where I messed up the formatting of some Python file:

It checks and fixes in one go, so next time I run it everything would be green again. That's something I disliked at first. I would have preferred some "check only" mode. But having used it for a while it's actually not an issue at all in my workflow. For most file types I have some "format on change" enabled anyways, but occasionally it does happen that something does not get formatted properly. Then I see that the "pre-commit" stage in CI is red and I want to run "fix" locally anyways. Really can't think of a reason why I would want a "check" only. It would always be followed by a "fix" anyways.

[ahans]

@ahans sorry I just merged main again into this branch. I keep accidentally pushing the damn button joy. I still can't believe I can merge stuff into your private repo scream. Sorry I did it twice pensive

No worries, I left the option checked that allows you (or any other maintainer) to make edits. I think it's fairly reasonable. Otherwise you would have to rely on me for everything (or copy my branch into your own and open up another PR).

[nachovizzo]

@ahans sorry I just merged main again into this branch. I keep accidentally pushing the damn button joy. I still can't believe I can merge stuff into your private repo scream. Sorry I did it twice pensive

No worries, I left the option checked that allows you (or any other maintainer) to make edits. I think it's fairly reasonable. Otherwise you would have to rely on me for everything (or copy my branch into your own and open up another PR).

Deswegen! Now I get it. Much easier to work like this. Thanks!

[nachovizzo]

How does a failure case look like? Is it clear if the failure is on black or any formatter? Would you mind providing a screenshot or something similar

Here's an example where I messed up the formatting of some Python file:

It checks and fixes in one go, so next time I run it everything would be green again. That's something I disliked at first. I would have preferred some "check only" mode. But having used it for a while it's actually not an issue at all in my workflow. For most file types I have some "format on change" enabled anyways, but occasionally it does happen that something does not get formatted properly. Then I see that the "pre-commit" stage in CI is red and I want to run "fix" locally anyways. Really can't think of a reason why I would want a "check" only. It would always be followed by a "fix" anyways.

If you do git commit locally, it will stop without changing the code. Assuming you've installed the hook pre-commit install Which I find terrific.

[nachovizzo]

I think is ready to merge @ahans . If you agree let me know because I need to change some config on the GH side

[ahans]

If you do git commit locally, it will stop without changing the code. Assuming you've installed the hook pre-commit install Which I find terrific.

I've never run pre-commit install. Been always a bit skeptical about git hooks. Maybe I should try it out! ;-)

I think is ready to merge @ahans . If you agree let me know because I need to change some config on the GH side

I reverted the clang-format version now and also changed the config for when to run the workflow to match the other ones. So yes, from my end I would say it's ready.

[ahans]

I also rebased onto latest main and dropped your merge commit. Just make sure to squash the two fixups into the one commit. But I think that happens automatically anyway.

[nachovizzo]

I also rebased onto latest main and dropped your merge commit. Just make sure to squash the two fixups into the one commit. But I think that happens automatically anyway.

yup, cuz I squash and merge always anyways