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

chore: update lints #2950

Merged
merged 2 commits into from
Feb 25, 2020
Merged

chore: update lints #2950

merged 2 commits into from
Feb 25, 2020

Conversation

wincent
Copy link
Contributor

@wincent wincent commented Feb 24, 2020

Updates to eslint-config-liferay v19.0.2 which brings in some new rules:

  • curly (enforces use of curlies around conditional blocks).
  • liferay/import-extensions (prohibits superflous extensions in imports).
  • liferay/array-is-array (enforces use of Array.isArray instead of instanceof Array).
  • liferay/sort-import-destructures (yet another in the family of "sorting" rules).
  • padding-line-between-statements (specifically, requiring a blank line before return statements, at Brian's request).
  • react/forbid-foreign-prop-types (so that we can be sure that stripping propTypes in production builds is safe; obviously won't be relevant in this repo).

and produces these new lint errors.

All of those have autofixes, so I applied them.

I also removed some cruft from the config file (values that are not needed because they are the defaults).

Note that in testing this, I discovered that the new "liferay/sort-import-destructures" rule and the @typescript-eslint/parser parser were not playing nicely together. I first tried updating some of the related NPM packages (ESLint, the parser etc) but that didn't work (although it did require me to update one config value), so I then dug deeper and tweaked the rule implementation to work with both the @typescript-eslint/parser and "espree" (ie. default) parsers:

@wincent
Copy link
Contributor Author

wincent commented Feb 24, 2020

Marking this as a draft @bryceosterhaus because it will conflict with #2947 — once that one is in I will just re-run the autofixes on this branch and update it. Expect the tests to fail because of the stale snapshots (fixed here).

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 24, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 752c699:

Sandbox Source
old-frog-thkyd Configuration
icy-feather-jl4dr Configuration

@wincent
Copy link
Contributor Author

wincent commented Feb 25, 2020

Rebased on top of #2947 (although I was wrong about it conflicting — it didn't) and taking out of draft status.

@wincent wincent marked this pull request as ready for review February 25, 2020 09:37
Updates to eslint-config-liferay v19.0.2 which brings in some new rules:

- curly (enforces use of curlies around conditional blocks).
- liferay/array-is-array (enforces use of `Array.isArray` instead of
  `instanceof Array`).
- liferay/import-extensions (prohibits superflous extensions in imports).
- liferay/sort-import-destructures (yet another in the family of
  "sorting" rules).
- padding-line-between-statements (specifically, requiring a blank line
  before `return` statements, at Brian's request).
- react/forbid-foreign-prop-types (so that we can be sure that stripping
  propTypes in production builds is safe; obviously won't be relevant in
  this repo).

and produces these new lint errors (about 67 of them):

https://gist.github.com/wincent/84bbd941eec684218acb21ad34fdb156

All of those have autofixes, so I applied them.

I also removed some cruft from the config file (values that are not
needed because they are the defaults).

Note that in testing this, I discovered that the
new "liferay/sort-import-destructures" rule and the
`@typescript-eslint/parser` parser were not playing nicely together. I
first tried updating some of the related NPM packages (ESLint, the
parser etc) but that didn't work (although it did require me to
update one config value), so I then dug deeper and tweaked the rule
implementation to work with both the `@typescript-eslint/parser` and
"espree" (ie. default) parsers:

liferay/eslint-config-liferay#153
@coveralls
Copy link

Pull Request Test Coverage Report for Build 4410

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage remained the same at 78.526%

Files with Coverage Reduction New Missed Lines %
packages/clay-modal/src/Hook.ts 1 50.0%
packages/clay-shared/src/useFocusManagement.ts 3 2.12%
Totals Coverage Status
Change from base Build 4408: 0.0%
Covered Lines: 2158
Relevant Lines: 2556

💛 - Coveralls

@wincent
Copy link
Contributor Author

wincent commented Feb 25, 2020

Two stalled jobs again:

That seem to have completed just fine:

Build time: 4m 12s. Total deploy time: 4m 12s

Build started at 11:54:39 AM and ended at 11:58:51 AM. Learn more about build minutes.

@bryceosterhaus
Copy link
Member

Tests are passing so that is good for me. The changes don't look like they would cause issue either.

Thanks for adding this!

@bryceosterhaus bryceosterhaus merged commit 2443bb1 into liferay:master Feb 25, 2020
@wincent wincent deleted the wincent/eslint-update branch February 26, 2020 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants