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

Scripts: Upgrade ESLint to the new major version (7.x) #22385

Closed
gziolo opened this issue May 15, 2020 · 3 comments · Fixed by #22771
Closed

Scripts: Upgrade ESLint to the new major version (7.x) #22385

gziolo opened this issue May 15, 2020 · 3 comments · Fixed by #22771
Assignees
Labels
Needs Dev Ready for, and needs developer efforts [Package] ESLint plugin /packages/eslint-plugin [Package] Scripts /packages/scripts [Status] In Progress Tracking issues with work in progress [Type] Breaking Change For PRs that introduce a change that will break existing functionality
Projects

Comments

@gziolo
Copy link
Member

gziolo commented May 15, 2020

Description

ESLint v7.0.0 was released:

https://eslint.org/blog/2020/05/eslint-v7.0.0-released

We should update our packages to use the latest version. It has a few breaking changes as highlighted here:
https://eslint.org/blog/2020/05/eslint-v7.0.0-released#breaking-changes

One of the issues resolved (eslint/eslint#11558) allows us to remove the following workaround added in #17744:

// Temporary workaround to use linting rules for both e2e and unit tests with all files
// until override files globbing logic is fixed when using with --config.
// @see https://github.com/eslint/eslint/issues/11558
'plugin:@wordpress/eslint-plugin/test-e2e',
'plugin:@wordpress/eslint-plugin/test-unit',

References

It might be helpful to look at previous PR where minor version upgrade for ESLint was applied: #21424. The changes included:

  • upgrade of ESLint package to the latest version
  • bringing all ESLint plugins and configs used to the latest version
  • listing all changes applied in the relevant changelog files
  • fixing any new errors that might pop up
@gziolo gziolo added Needs Dev Ready for, and needs developer efforts [Package] Scripts /packages/scripts [Package] ESLint plugin /packages/eslint-plugin [Type] Breaking Change For PRs that introduce a change that will break existing functionality labels May 15, 2020
@gziolo gziolo changed the title ESLint Plugin: Upgrade ESLint to the new major version (7.x) Scripts: Upgrade ESLint to the new major version (7.x) May 15, 2020
@gziolo gziolo added this to To do in Core JS May 15, 2020
@aduth
Copy link
Member

aduth commented May 18, 2020

Somewhat related: #16795 (comment)

@ocean90
Copy link
Member

ocean90 commented May 31, 2020

@gziolo In #22771 I have started with the upgrade. We may have to wait for two dependencies getting their update to avoid the warning.

There are some new JS lint errors which seem to be valid. Can you confirm that they are correct and should be fixed? I suppose a separate PR is preferred?

@gziolo
Copy link
Member Author

gziolo commented Jun 1, 2020

There are some new JS lint errors which seem to be valid. Can you confirm that they are correct and should be fixed? I suppose a separate PR is preferred?

@ocean90, thank you for starting this PR. I left my detailed comment there. I think those are valid issues that need to be addressed and we definitely need to split into more PRs. How we proceed, it's up for debate :)

@gziolo gziolo moved this from To do to Done in Core JS Jun 4, 2020
@gziolo gziolo moved this from Done to In progress in Core JS Jun 4, 2020
@gziolo gziolo moved this from In progress to Done in Core JS Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Ready for, and needs developer efforts [Package] ESLint plugin /packages/eslint-plugin [Package] Scripts /packages/scripts [Status] In Progress Tracking issues with work in progress [Type] Breaking Change For PRs that introduce a change that will break existing functionality
Projects
No open projects
Core JS
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants