-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[eslint] make CI lint tests, utils, resolvers, ...etc #1475
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
Conversation
node_modules | ||
# we want to ignore "tests/files" here, but unfortunately doing so would | ||
# interfere with unit test and fail it for some reason. | ||
# tests/files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also #1413 (comment) :
There is a technical problem: Since we don't want to lint
tests/files
, an.eslintignore
file needs to be added. But doing that alters the behavior of unit tests forno-unused-modules
, the rule will considertests/files/**/*
to be ignored, hence failing the tests.
The current approach has the disadvantage that contributors 1. cannot run I found another approach: add the file I am going to force push with the later approach. |
64072d1
to
e982c78
Compare
I think the latter approach you suggest sounds better. |
37ce976
to
a53bf3d
Compare
We shouldn't be autofixing warnings; looks like the second commit is only doing that. I'll rebase and clean it up a bit, thanks! <3 |
f7716ca
to
c3cad51
Compare
First steps for fixing #1413.