-
-
Notifications
You must be signed in to change notification settings - Fork 675
Description
When you run tools/test and one of the files that was touched was a file we exclude in our .eslintignore, it fails with an ESLint error.
This doesn't affect CI (with tools/test --all-files); only the interactive tools/test, including --diff.
As discussed at #4430 (comment), this is basically due to a bug in the interaction of tools/test and eslint with how tools/test handles being selective. Here's the ESLint command line:
$ bash -x tools/test lint
…
+ eslint --max-warnings=0 src/__flow-tests__/nav-test.js src/react-navigation.js src/sharing/ShareToPm.js src/sharing/ShareToStream.js src/sharing/SharingScreen.js
/home/greg/z/mobile/src/__flow-tests__/nav-test.js
0:0 warning File ignored because of a matching ignore pattern. Use "--no-ignore" to override
So it's complaining because it was specifically asked to look at that particular file, and the file is ignored. That's a reasonable behavior for ESLint; it could be helpful in an actual interactive command-line context, even though from a script like this it's just an annoying complication.
As I wrote then:
Ideally we'd persuade
eslintto just apply the.eslintignoreand not worry about how specifically we mentioned a file that happens to be covered by it. Failing that, a possible solution thattools/testcould do would be to somehow parse and apply the.eslintignore, to filter out such files before mentioning them to ESLint. But that'd probably be more work than this issue is worth to us.
But I've been running into this more, recently, as I've been adding types-tests and working on some of our files that have types-tests, because those types-tests are among the few categories of files we exclude. It's particularly annoying when running something like git rebase --exec 'tools/test lint flow --diff @~' to check each commit in a branch, because the failures cause the rebase --exec loop to stop.
So I think this is worth dealing with after all.