-
-
Notifications
You must be signed in to change notification settings - Fork 140
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
Fix prettier, eslint and stylelint config #530
base: main
Are you sure you want to change the base?
Fix prettier, eslint and stylelint config #530
Conversation
Upgrade prettier to latest supported Downgrade eslint-config-prettier to version that supports eslint v6 Add npm format scripts for prettier Exclude coverage and dist from prettier execution Update lint-staged to run prettier
We're using the main husky package, not Yorkie
As recommended by eslint-config-prettier-check
✅ Deploy Preview for creativecommons-chooser ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Run full lint format and build
I'm not aware of a general solution for a containerised application with git hooks but from a quick look:
|
@dijitali after considering this, I'm thinking the best option you've outlined is likely just It's a tradeoff for sure, but it does mean that there's less likelihood of things failing. We don't automate prettier in other repositories, so I think there's enough of a pattern there for it not to be an outlier in that regard. |
Yep, makes sense I think 👍 Am aware that this repo is due/blocked for a major upgrade (#356) which would perhaps negate the fiddly docker requirements. However given that this project is referenced to as an exemplary javascript project in the guidelines and that this repo already has some partially functional config, would it be good to just get the prettier/eslint config in a functional state and leave out the automatic execution? |
Yeah, I think improvements here, even if automating them isn't reasonable, are still worth doing until the project is upgraded, or refactored in other ways. |
Automatic styling/linting via git hooks is problematic with the current dockerised environment
@possumbilities - think this is all set for a re-review when you have a chance 🙏 Oh and if you had the git hooks installed on your local machine, Husky v8.0.3 docs say to remove them with: git config --unset core.hooksPath |
Fixes
Description
Fixes prettier, eslint and stylelint config. Now run manually via
npm run lint
andnpm run format
as discussed in #530 (comment).Technical details
Add and configure husky to run lint-staged (latest v8.x, since v9 drops node v14 support):npm install --save-dev [email protected]
npx husky add .husky/pre-commit npx lint-staged
Upgrade Prettier and add
npm run format
scripts:npm update prettier
Downgrade
eslint-config-prettier
to be compatible with eslint v6 (current v7.2.0 requires eslint upgrade):npm install --save-dev [email protected]
Ignores
coverage
anddist
directories when running PrettierUpdate
lint-staged
to run prettierRemove git-hooks config from
package.json
(this project uses Husky not Yorkie)Check eslint <-> prettier conflicts and remove problematic rules:
Set
lint-staged
to runstylelint
on the same file patterns as the npm script is configured to. This potentially needs further work as a separate PR because currently onmain
, if you runnpx stylelint **/*.{css,vue} --fix
(the equivalent of whatlint-staged
runs it against), there's a bunch of things flagged in thestatic
directory.Tests
npm run lint
andnpm run format
and check that tests and builds still work OKChecklist
Update index.md
).main
ormaster
).visible errors.
Developer Certificate of Origin
For the purposes of this DCO, "license" is equivalent to "license or public domain dedication," and "open source license" is equivalent to "open content license or public domain dedication."
Developer Certificate of Origin