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.

Sign up for GitHub

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

Jump to bottom

Adds checkmake repo #756

Merged
merged 1 commit into from
Dec 19, 2022
Merged

Adds checkmake repo #756

merged 1 commit into from
Dec 19, 2022

Conversation

colindean
Copy link
Contributor

@colindean colindean commented Dec 11, 2022 •
edited by asottile
Loading

checkmake got a hook configuration in mrtazz/checkmake@4e37f3a

my new repository:

  • is added to the bottom or with existing repos from the same account
  • contains a license
  • is not language: system or language: docker
  • does not contain "pre-commit" in the name

@colindean
Adds checkmake repo
9ea7379 
checkmake got a hook configuration in mrtazz/checkmake@4e37f3a
@asottile
Copy link
Member

pass_filenames: false is incorrect I believe -- the tool should operate on files passed to it

@colindean
Copy link
Contributor Author

Where are you seeing that?

Looks like it's set to true here.

@asottile
Copy link
Member

ah I think I just misread -- pass_filenames: true is the default so you shouldn't need to set it

is there a reason that types: [makefile] wasn't used?

colindean added a commit to colindean/checkmake that referenced this pull request Dec 19, 2022
@colindean
Reduces files to the built-in makefile type for pre-commit
f874f36 
As nudged [here][1], but this won't be fully effective until pre-commit/identify#353 is merged.

I think this is safe because so few people use `*.make`... I think I've seen it maybe a few times in 20 years.

[1]: pre-commit/pre-commit.com#756 (comment)
@colindean colindean mentioned this pull request Dec 19, 2022
Merged
5 tasks
@colindean
Copy link
Contributor Author

Looks like *.make, declared here, is missing from the extension list here but the filenames are listed here adequately.

I've proposed adding .make in pre-commit/identify#353 and switching to the types property in mrtazz/checkmake#77. It might be a hot minute before 353 makes it into a pre-commit release but I think that's OK because I've rarely seen .make and the change in checkmake is so recent that I doubt that anyone who's adopted the pre-commit hook already relies on it.

@asottile
Copy link
Member

identify has been released so you should be able to use types now

asottile
asottile approved these changes Dec 19, 2022
View reviewed changes
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

@asottile asottile merged commit 2b35c70 into pre-commit:main Dec 19, 2022
@colindean colindean deleted the patch-1 branch January 19, 2023 01:38
@trinitronx trinitronx mentioned this pull request Jul 14, 2024
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asottile asottile asottile approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@colindean @asottile