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

build: update eslint #1894

Closed
wants to merge 1 commit into from
Closed

Conversation

WikiRik
Copy link
Member

@WikiRik WikiRik commented Jan 7, 2022

ESLint updated and made sure it only runs on the CI check for Node 14. All fixes were auto-updated or ignored for now.
The following are ignored:

  • src/lib/alpha.js (no-misleading-character-class on lines 32 and 38, I don't speak Arabic but these might be false positives)
  • src/lib/isIdentityCard.js (max-len on lines 24 and 309, this can be refactored)
  • src/lib/util/merge.js (default-param-last on line 2, this can be refactored)
  • test/client-side.js (import/no-unresolved and import/extensions on line 3 and 4, these are fixed after running npm run build)
  • test/exports.js (import/no-unresolved and import/extensions on line 3, these are fixed after running npm run build)

The ignored files that can be refactored will be done as a different PR to keep this PR relatively small.

I didn't notice #1869 before, but I think that this PR is still useful to reduce the changes for the other PR.

Once we run CI on Node 12 and up we can put npm run lint back to the pretest script and remove the additional test, although I like that the tests are not dependent on the linter.

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable) Not applicable
  • Tests written (where applicable) Not applicable

@codecov
Copy link

codecov bot commented Jan 7, 2022

Codecov Report

Merging #1894 (f048a4c) into master (f055c11) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1894   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          102       102           
  Lines         2072      2071    -1     
  Branches       472       472           
=========================================
- Hits          2072      2071    -1     
Impacted Files Coverage Δ
src/lib/alpha.js 100.00% <ø> (ø)
src/lib/isDataURI.js 100.00% <ø> (ø)
src/lib/isEAN.js 100.00% <ø> (ø)
src/lib/isHSL.js 100.00% <ø> (ø)
src/lib/isIMEI.js 100.00% <ø> (ø)
src/lib/isURL.js 100.00% <ø> (ø)
src/lib/util/merge.js 100.00% <ø> (ø)
src/lib/isBase64.js 100.00% <100.00%> (ø)
src/lib/isCurrency.js 100.00% <100.00%> (ø)
src/lib/isDate.js 100.00% <100.00%> (ø)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f055c11...f048a4c. Read the comment docs.

@profnandaa profnandaa added the blocked For PRs that are blocked due to pending discussions, etc. label Apr 22, 2022
@profnandaa
Copy link
Member

Thanks for your suggestion but I think this change is not necessary for now. We can hold off on it for now.

@profnandaa profnandaa closed this Apr 22, 2022
@WikiRik
Copy link
Member Author

WikiRik commented Apr 22, 2022

Sure, thanks for looking at it!

@WikiRik WikiRik deleted the eslint-update branch April 22, 2022 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked For PRs that are blocked due to pending discussions, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants