-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add JSHint ESLint config #12840
Add JSHint ESLint config #12840
Conversation
Not certain how I feel about adding this to the package, if it's for legacy purpose, I propose using it in WordPress directly in addition to the eslint package, instead of adding it to the reusable package |
I included it in the package as for any existing plugin, theme, or core that is looking to update their build tooling to switch from JSHint to ESLint can do so using a backwards compatible set of rules. The rules themselves in the configuration are not all similarly named so without some knowledge of JSHint and ESLint making such a ruleset configuration would not be a trivial task. This same approach would be required to have to first disable all the unrequired rules included in I fully plan to use this ruleset myself in the plugins I'm associated with whilst the teams get familiar with ESLint work can begin on migrating to the full |
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.
I think we should clarify when jshint
config should be used. Otherwise I think the explanation @ntwb shared makes a lot of sense. We can remove this config when introducing next breaking change as it looks like a temporary addition.
Co-Authored-By: ntwb <[email protected]>
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.
Code wise looks good to me. It just needs the decision whether we want to have it included. I am personally fine with having it introduced.
You all convinced me, I'm fine to include now. We can remove it later if needed. |
* Add ESLint JSHint config * Update packages/eslint-plugin/README.md Co-Authored-By: ntwb <[email protected]>
* Add ESLint JSHint config * Update packages/eslint-plugin/README.md Co-Authored-By: ntwb <[email protected]>
Description
This pull request is a follow up to #12763 and seeks to add a new
jshint
ESLint shared configuration of the existing WordPress.jshintrc
file.This pull request makes available a "drop-in" ESLint JSHInt configuration that will allow WordPress to easily transition away from JSHint to ESLint per the #WP31823 ticket whilst the significant legacy JavaScript is updated to adhere to the WordPress JavaScript Coding Standards in the
@wordpress/eslint-plugin/recommended
ESLint configuration.How has this been tested?
Screenshots
Types of changes
Checklist: