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

Make Makefile argument optional for easier use from tools. #81

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

michael-delarue-axomic
Copy link

This seems to solve problems with running checkmake from pre-commit. It very simply makes the Makefile argument optional which means that checkmake doesn't seem to run.
(see also #80 and some comments on other (closed) issues (e.g. #69)

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.

  • [x ] CI passes
  • [x ] Description of proposed change
  • [x ] Documentation (README, docs/, man pages) is updated
  • [x ] Existing issue is referenced if there is one
  • [!! ] Unit tests for the proposed change

no unit test yet since this basically does nothing. Might investigate learning how to do that if considered worthwhile.

@michael-delarue-axomic
Copy link
Author

According to the pre-commit documentation for adding new hooks the default is not to run hooks when there are no files to be checked and the checkmake hook configuration does not override this, so I am not really sure why this is helping, but it has.

@colindean
Copy link
Contributor

Can you show the output you're seeing when it fails? I've used checkmake at rev c047d51 for many months now without a problem across more than a dozen repos.

@mrtazz
Copy link
Owner

mrtazz commented Aug 17, 2023

it would be good to have a more detailed description here of what exactly fails and how this is fixing it. Especially since there are reports where things are working.

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.

3 participants