wp-calypso
wp-calypso copied to clipboard
chore: enable ESLint autofix in pre-commit hook.
Proposed Changes
This PR adds the --fix
parameter to the pre-commit hook ESLint in an attempt to automatically fix warnings.
Context:
Our Check code style
build shows a persistent number of warnings and errors that could have been fixed automatically. For instance, this file has a warning that can be auto-fixed by ESLint but the warning persists in our codebase, adding to the number of issues in the Check Code Style build.
Testing Instructions
Ensure the following build configurations are passing:
- [ ] Code Style
- [ ] Unit Tests
Manually test the changes:
- make changes to a file that will result in a autofixable warning (eg. jsdoc/multiline-blocks)
- add file to staging.
- commit changes.
The warning should be autofixed.
Pre-merge Checklist
- [ ] Have you written new tests for your changes?
- [ ] Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
- [ ] Have you checked for TypeScript, React or other console errors?
- [ ] Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
- [ ] Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
Related to #
This PR does not affect the size of JS and CSS bundles shipped to the user's browser.
Generated by performance advisor bot at iscalypsofastyet.com.
I wouldn't do this: many autofixes are known to be buggy (I personally had issues with the jsdoc
rules) or approximate. They require manual inspection before committing. Unlike Prettier formatting, they are not reliable enough to be done automatically.
Plus, with eslint autofixing on save (especially with the new "recommended settings" here: p4TIVU-agZ-p2), hopefully this is less of an issue in the future
I wouldn't do this: many autofixes are known to be buggy (I personally had issues with the jsdoc rules) or approximate. They require manual inspection before committing. Unlike Prettier formatting, they are not reliable enough to be done automatically.
This is good feedback, thanks! It did occur to me that this might be a big change without necessarily producing the expected results.
Plus, with eslint autofixing on save (especially with the new "recommended settings" here: p4TIVU-agZ-p2), hopefully this is less of an issue in the future
This relies on the user using VSCode or one of its derivatives, which isn't guaranteed. If that were the case, I would expect the error and warning count to stay consistent or at least stay relatively constant whereas the count is steadily creeping up.
I then used "save without formatting" in VS Code so that it wouldn't auto-format there. I also ran yarn postinstall to "re-install" the husky hooks. And finally, git commit -a -m "test" on this branch, but it didn't auto-format the file.
Odd - it seems to have worked for me while I was trying this out.
In any case, I think this PR led to a good exchange of ideas. I'll close this for now and revert to the manual approach to fix warnings and errors for now.