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

Detect ScalaFmt config as a scala file #436

Closed
wants to merge 1 commit into from
Closed

Detect ScalaFmt config as a scala file #436

wants to merge 1 commit into from

Conversation

danio
Copy link

@danio danio commented Dec 7, 2023

Add ScalaFmt config so that pre-commit hooks will check changes to the config file and get latest

Currently when I run precommit on a scala repository using a hook that has:
types: [scala]
set in .pre-commit-hooks.yaml and I have local edits to my .scalafmt.conf file, precommit doesn't run the hook script as it doesn't find any scala files that have changed.

Identifying .scalafmt.conf as a scala file will fix this

@@ -31,7 +31,7 @@
'cmake': {'text', 'cmake'},
'cnf': {'text'},
'coffee': {'text', 'coffee'},
'conf': {'text'},
'conf': {'text', 'ini'},
Copy link
Member

Choose a reason for hiding this comment

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

this is a breaking change and not generally correct since we can't guarantee the structure of a genetic extension like "conf"

@danio
Copy link
Author

danio commented Dec 7, 2023 via email

@asottile
Copy link
Member

asottile commented Dec 7, 2023

When would a .conf file ever be anything other than a configuration file?

On Thu, 7 Dec 2023 at 18:17, Anthony Sottile @.> wrote: @.* commented on this pull request. ------------------------------ In identify/extensions.py <#436 (comment)>: > @@ -31,7 +31,7 @@ 'cmake': {'text', 'cmake'}, 'cnf': {'text'}, 'coffee': {'text', 'coffee'}, - 'conf': {'text'}, + 'conf': {'text', 'ini'}, this is a breaking change and not generally correct since we can't guarantee the structure of a genetic extension like "conf" — Reply to this email directly, view it on GitHub <#436 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIQ5QOAEFQWJZ2R7DHJ3TYIIB5LAVCNFSM6AAAAABALM7BY6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONZQG44TAMRWGA . You are receiving this because you authored the thread.Message ID: @.***>

I think you're confused -- you've set .conf as ini

@danio
Copy link
Author

danio commented Dec 7, 2023

When would a .conf file ever be anything other than a configuration file?

On Thu, 7 Dec 2023 at 18:17, Anthony Sottile @.> wrote: _@**.**_* commented on this pull request. --------->
I think you're confused -- you've set .conf as ini

no need to be patronising!

reverted the change, but I think a config file type would be a useful concept, then ini would be a type of config and it would be clearer

@asottile asottile closed this Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants