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

Use wordpress/es6 ESLint config #5600

Merged
merged 3 commits into from
Mar 21, 2018
Merged

Use wordpress/es6 ESLint config #5600

merged 3 commits into from
Mar 21, 2018

Conversation

ntwb
Copy link
Member

@ntwb ntwb commented Mar 14, 2018

Description

This PR migrates ESLint ES6 rules out of Gutenberg and into the eslint-plugin-wordpress package and the wordpress/es6 configuration.

This PR has a companion PR here WordPress-Coding-Standards/eslint-plugin-wordpress#137

How Has This Been Tested?

Proof of concept at the moment, testing to follow

Screenshots (jpeg or gifs if applicable):

Types of changes

Non-breaking change to the ESLint configuration

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@ntwb ntwb added [Type] Task Issues or PRs that have been broken down into an individual action to take [Status] In Progress Tracking issues with work in progress [Type] Build Tooling Issues or PRs related to build tooling labels Mar 14, 2018
@ntwb ntwb self-assigned this Mar 14, 2018
@ntwb ntwb requested a review from gziolo March 14, 2018 05:59
eslint/config.js Outdated
@@ -2,6 +2,7 @@ module.exports = {
parser: 'babel-eslint',
extends: [
'wordpress',
'wordpress/es6',
Copy link
Member

Choose a reason for hiding this comment

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

As discussed previously we might want to proceed with es-next to avoid confusion with all the recent versions of the JS language.

@@ -39,7 +40,6 @@ module.exports = {
'comma-spacing': 'error',
'comma-style': 'error',
'computed-property-spacing': [ 'error', 'always' ],
'constructor-super': 'error',
Copy link
Member

Choose a reason for hiding this comment

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

computed-property-spacing is also es6 specific. Is there a way to identify whart also belong to es6?

Copy link
Member Author

Choose a reason for hiding this comment

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

The ESLint rule list has a ES6 category of rules: https://eslint.org/docs/rules/#ecmascript-6

Copy link
Member

Choose a reason for hiding this comment

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

Ah okey, that explains everything 😃

@gziolo
Copy link
Member

gziolo commented Mar 18, 2018

It feels like all non-react specific rules should go to es-next configuration or however this grouping should be called :)

@ntwb
Copy link
Member Author

ntwb commented Mar 19, 2018

@gziolo Done, this is ready for review and merge 👍

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I tested all updated rules. All good. Thanks for taking care of esnext rules 👍

@gziolo gziolo merged commit 030b562 into master Mar 21, 2018
@gziolo gziolo deleted the try/eslint-wordpress-es6 branch March 21, 2018 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Type] Build Tooling Issues or PRs related to build tooling [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants