-
-
Notifications
You must be signed in to change notification settings - Fork 967
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
Run prettier
on JavaScript via pre-commit
#1838
Run prettier
on JavaScript via pre-commit
#1838
Conversation
448add0
to
4ea09b4
Compare
I added a commit to show that |
@@ -1,4 +1,4 @@ | |||
exclude: '(^djangoproject\/static\/.*$)' | |||
exclude: '^djangoproject\/static\/(css\/|fonts\/|img\/|robots).*$' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes pre-commit
only run on the js
directory in static
. In a series of future PRs I'd like to remove this exclude
line and fix all of the warnings generated by the other directories.
@@ -19,6 +19,7 @@ repos: | |||
- id: debug-statements | |||
- id: detect-private-key | |||
- id: end-of-file-fixer | |||
exclude: '(^djangoproject\/static\/js\/lib\/.*$)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to modify minified JavaScript files to add a newline at the end.
@@ -46,6 +47,7 @@ repos: | |||
hooks: | |||
- id: prettier | |||
exclude_types: [html, json, scss] | |||
exclude: '(^djangoproject\/static\/js\/lib\/.*$)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to run the formatter on minified JavaScript files.
"options": { | ||
"tabWidth": 2, | ||
"singleQuote": true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These options are my personal preference. Let me know if this should be modified.
This patch removes the global `djangoproject/static/*` exclude rule in `.pre-commit-config.yaml` to enable `prettier` to run on this project's JavaScript files. The `djangoproject/static/js/lib/` directory is now excluded in the `prettier` hook because there is no value in reformatting a minified JavaScript file. I also added a `.prettierrc` file to the root of the project to control JavaScript formatting.
1ffb7f1
to
091ca2c
Compare
for more information, see https://pre-commit.ci
You can see in 0091a87 that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, I'm very happy about having a an automatically enforced standard now 🎸
This patch removes the global
djangoproject/static/*
exclude rule in.pre-commit-config.yaml
to enableprettier
to run on this project's JavaScript files. Thedjangoproject/static/js/lib/
directory is now excluded in theprettier
hook because there is no value in reformatting a minified JavaScript file.I also added a
.prettierrc
file to the root of the project to control JavaScript formatting.@bmispelon Is this what you had in mind as a requirement to get #1835 in?