Skip to content

/ gutenberg Public

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

Jump to bottom

ESLint Plugin: Update eslint jsdoc dependency #34338

Merged
merged 7 commits into from Sep 8, 2021
Merged

ESLint Plugin: Update eslint jsdoc dependency #34338

merged 7 commits into from Sep 8, 2021

Conversation

@renatho
Copy link
Contributor

@renatho renatho commented Aug 26, 2021 •
edited

Fixes #34297

Description

  • It updates the eslint-plugin-jsdoc in order to solve the reported issue.
  • It also disables some rules that started failing after the update that wouldn't be easy to fix.

How has this been tested?

npm run lint-js in Gutenberg repo.

Types of changes

Breaking change

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
@gziolo
Copy link
Member

@gziolo gziolo commented Aug 27, 2021 •
edited

Nice one. Thank you for taking care of the reported issue so quickly.

Should we consider it a breaking change for the ESLint plugin package (it would also cascade to @wordpress/scripts which use the recommended ESLint settings)? Are new rules set on the error level or they are just warnings?

@gziolo gziolo added this to Needs review in Core JS Aug 27, 2021
@renatho
Copy link
Contributor Author

@renatho renatho commented Aug 27, 2021 •
edited

Nice one. Thank you for taking care of the reported issue so quickly.

Should we consider it a breaking change for the ESLint plugin package (it would also cascade to @wordpress/scripts which use the recommended ESLint settings)? Are new rules set on the error level or they are just warnings?

Thank you for the quick review! You're right, it should probably be considered as a breaking change (updated in the PR description).
For Gutenberg repo, with the update rules, it's with the same warnings, but it can have different impact in other projects (and we also changed some rules, like disabling the jsdoc/valid-types).

@renatho renatho force-pushed the update/eslint-jsdoc-dependency branch from ffec4db to 53bb848 Sep 3, 2021
@renatho renatho requested a review from gziolo Sep 3, 2021
gziolo
gziolo approved these changes Sep 6, 2021
View changes
Copy link
Member

@gziolo gziolo left a comment

I included a commit that adds an entry to the CHANGELOG file for @wordpress/eslint-plugin package and this PR should be good to go when CI turns green. Nice work, thank you for going through all the feedback shared 🙇🏻

@gziolo
Copy link
Member

@gziolo gziolo commented Sep 6, 2021

It looks like this branch will need a rebase before we can proceed.

Renatho De Carli Rosa and others added 6 commits Sep 8, 2021
Update eslint-plugin-jsdoc dependency
e395231
Fix JSDoc comment
71807e9
Update eslint rules disabling some of them
0c85f95 
It disables some rules that are failing after the eslint plugin
update.
Update no-multi-asterisks rule options
4a1764e
Enable and fix jsdoc/valid-types eslint rule
975addb
@gziolo
Update CHANGELOG.md
596512a
@renatho renatho force-pushed the update/eslint-jsdoc-dependency branch from 40cc88d to 596512a Sep 8, 2021
Fix jsdoc line alignment
d941a6d
@renatho
Copy link
Contributor Author

@renatho renatho commented Sep 8, 2021

It looks like this branch will need a rebase before we can proceed.

Thank you for the review @gziolo! =)
Branch rebased and fixed a new warning after the rebase: d941a6d

@gziolo
Copy link
Member

@gziolo gziolo commented Sep 8, 2021

🎉 Let's merge when CI turns green 👍🏻

@gziolo
Copy link
Member

@gziolo gziolo commented Sep 8, 2021

I see CI failures for the RN checks so we can ignore them here.

@gziolo gziolo changed the title Update eslint jsdoc dependency ESLint Plugin: Update eslint jsdoc dependency Sep 8, 2021
@gziolo gziolo merged commit b220881 into WordPress:trunk Sep 8, 2021
17 of 19 checks passed
@github-actions github-actions bot added this to the Gutenberg 11.6 milestone Sep 8, 2021
@gziolo gziolo moved this from Needs review to Done in Core JS Sep 8, 2021
@renatho renatho deleted the update/eslint-jsdoc-dependency branch Sep 8, 2021
fullofcaffeine added a commit that referenced this issue Sep 9, 2021
@fullofcaffeine
Merge branch 'trunk' into replace/confirm-et-al
f99c357 
* trunk: (214 commits)
  Fix snackbar overflow on nav editor (#34661)
  Mobile - Allow disabling text and background color via theme.json (#34633)
  Fix disabled blocks logical error on Widgets screen (#34634)
  [Mobile] - Global styles - Add support to render font sizes and line height (#34144)
  ESLint Plugin: Update eslint jsdoc dependency (#34338)
  Scripts: Add CHANGELOG entry for `jest-dev-server` upgrade (#34657)
  Bump jest-dev-server to v5 (#34560)
  Refactor the `core-data` store to thunks (#28389)
  Only capture toolbars on parent Nav block when not in vertical mode (#34615)
  Update Changelog for 11.5.0-rc.1
  Bump plugin version to 11.5.0-rc.1
  Gallery block: Fix media placeholder height in site editor (#34629)
  Border Controls: Display color indicator and check selected color (#34467)
  Remove horizontal and vertical navigation block variations from inserter (#34614)
  AlignmentMatrixControl : Fix/update docs (#34624)
  Gap block support: force gap change to cause the block to re-render (fix Safari issue) (#34567)
  [Block Library - Social Links]: Use the new `flex` layout (#34493)
  [Mobile] Update the bottom sheet header (#34309)
  Group block: Add a row variation (#34535)
  Migrate entities.js to thunks (#34582)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gziolo gziolo

@ajitbohra ajitbohra

@nerrad nerrad

@ntwb ntwb

@chrisvanpatten chrisvanpatten

@ellatrix ellatrix

Assignees
No one assigned
Labels
[Package] ESLint plugin [Package] Scripts [Type] Enhancement
Projects
Core JS
  
Done
Milestone
Gutenberg 11.6
Linked issues

Successfully merging this pull request may close these issues.

Scripts: wp-scripts lint-js Breaks After v16.1.4
2 participants
@renatho @gziolo