Skip to content

Adds checkmake repo#756

Merged
asottile merged 1 commit intopre-commit:mainfrom
colindean:patch-1
Dec 19, 2022
Merged

Adds checkmake repo#756
asottile merged 1 commit intopre-commit:mainfrom
colindean:patch-1

Conversation

@colindean
Copy link
Copy Markdown
Contributor

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

checkmake got a hook configuration in checkmake/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 checkmake/checkmake@4e37f3a
@asottile
Copy link
Copy Markdown
Member

asottile commented Dec 15, 2022

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

@colindean
Copy link
Copy Markdown
Contributor Author

colindean commented Dec 17, 2022

Where are you seeing that?

Looks like it's set to true here.

@asottile
Copy link
Copy Markdown
Member

asottile commented Dec 17, 2022

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
Copy Markdown
Contributor Author

colindean commented Dec 19, 2022

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 checkmake/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
Copy Markdown
Member

asottile commented Dec 19, 2022

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
Copy Markdown
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