Do not fail yarn linc for ignored file warning (#11615)
#11641
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Hmm. What if the changed files are in an ESLint-ignored folder? I guess in that case we'd still see this issue, wouldn't we? I think the right fix for this would have been to disable this particular rule. At least when running on changed files. |
|
Or maybe we should still exit with the zero code if the only failing rule is this one? I think printing this information is still useful. Assuming my analysis in #11641 (comment) is right. If it's not let me know! |
HeroProtagonist
force-pushed
the
lint-hidden-files-#11615
branch
from
5e299d7
to
fe3414c
Compare
It would display this very similar issue
From my looking into eslint, it seems this cannot be disabled because it is not so much a rule as it is eslint being told one thing and then supplied another.
This is what the code now reflects. I made a function that will validate the warnings from the eslint report. If all the warnings contain the |
|
Sorry--do you mind rebasing on top of master? Thanks! |
HeroProtagonist
force-pushed
the
lint-hidden-files-#11615
branch
from
fe3414c
to
8c9784f
Compare
HeroProtagonist
changed the title
yarn linc for ignored file warning (#11615)
|
Done! |
scripts/eslint/index.js
Outdated
| const report = cli.executeOnFiles(filePatterns); | ||
| const formattedResults = formatter(report.results); | ||
| console.log(formattedResults); | ||
| return report; | ||
| } | ||
|
|
||
| function validWarnings(report) { |
There was a problem hiding this comment.
Let's move this to where it's used?
Also let's rename to something like getSourceCodeWarnings.
In fact I would just inline this logic in the script that uses it, and add a comment explaining why it exists.
There was a problem hiding this comment.
Changes reflect this. Do you prefer to have function declaration vs definition?
|
Thanks! |
Resolves #11615
Problem
This error happens because by default eslint ignores dotfiles. The
yarn linccommand can supply dotfiles like.eslintrc.jscausing this error because of the mismatch between the ignore patterns and what is being given as lint input. This eslint thread talks about this issue with this PR resolving it.Solution
Hidden files will now be linted, but not hidden folders. As of now the only hidden
.jsfile seems to be the.eslintrc.js, but this will prevent this from happening if any are added in the future. As a bonus thercfile will be linted going forward.Happy Thanksgiving! 馃