mrtazz / checkmake

experimental linter/analyzer for Makefiles
MIT License
1.04k stars 45 forks source link

Add pre commit hook #69

Closed trinitronx closed 2 years ago

trinitronx commented 2 years ago

Checklist

Not all of these might apply to your change but the more you are able to check the easier it will be to get your contribution merged.

colindean commented 2 years ago

This is great! Thanks for doing this up.

trinitronx commented 2 years ago

Ping @mrtazz

richtong commented 2 years ago

Yes please ccept the PR :-)

renefritze commented 2 years ago

It seems the hook will currently not work as documented when run from pre-commit run -a because no file is then passed to checkmake. If I edit the config to

-   repo: https://github.com/trinitronx/checkmake/
    rev: 0.2.2
    hooks:
    -   id: checkmake
        files: Makefile

then that mode also works. Maybe this is something you'd want to document or maybe there's a smart way to handle this on the hook side?

mrtazz commented 2 years ago

thanks for taking the time to contribute!

trinitronx commented 4 months ago

[...SNIP...] because no file is then passed to checkmake

@renefritze: Sorry for the belated reply (too many GitHub notifications were cluttering my inbox for years).

Yes, this was fixed in a12e9a0 and 2d14c7a which also slightly depended on #70 being merged as well for multiple files support (which the -a / --all-files flag would pass).

I noticed that you were using the "0.2.2" pre-release testing tag in my fork, which was prior to both of those changes I mentioned above. This tag in my forked repo was meant mostly for my own internal testing, as well as anyone who wanted to alpha/beta test these changes prior to my pull requests being merged upstream (here on this repo). You may have realized this by now, but if you used the 0.2.3 or later tags from my fork, it probably would have worked. However, now in 2024, everything seems to be working well using the official 0.2.2 tag on this repo (mrtazz/checkmake@0.2.2).

Also, many thanks to @colindean for pushing the rest of the needed changes for Makefile .make file extension support, and the checkmake hook support upstream to this repo, pre-commit, and pre-commit/identify projects! :shipit: :package: :rocket: Coordinating changes across so many repos is a no small feat of Open Source collaboration, and is greatly appreciated! Thanks! :pray:

For more details, see:

I've revisited using this hook again in 2024, and it appears that all the necessary bits across all these projects are now merged and released! :rocket:

After a pre-commit gc, pre-commit clean, and pre-commit autoupdate for an old project, everything works well using this official repo's 0.2.2 tag!

:tada: :champagne: Thanks @mrtazz!