fix: should exit with an error if no files were checked #58
Conversation
|
This is a good PR. I'm going to look at this again tomorrow. Thanks, @brandonocasey! |
There was a problem hiding this comment.
Thanks, @brandonocasey!
The thoughtful check on line 118 to 121 should stay!
- add a test (lemme know if you'd like assistance with the test).
index.js
Outdated
| @@ -106,27 +106,34 @@ prog | |||
| const acornOpts = { ecmaVersion: e, silent: true } | |||
| if (esmodule) { acornOpts.sourceType = 'module'} | |||
|
|
|||
| let globbedFiles = []; | |||
index.js
Outdated
| file, | ||
| } | ||
| errArray.push(errorObj) | ||
| globbedFiles = globbedFiles.concat(glob.sync(pattern, globOpts)); |
There was a problem hiding this comment.
Remove changed to lines 115 and 116.
index.js
Outdated
| globbedFiles = globbedFiles.concat(glob.sync(pattern, globOpts)); | ||
| }); | ||
|
|
||
| if (globbedFiles.length === 0) { |
|
I think I made all the changes requested, but I am not totally sure. I also added two tests, and I had to change one of the existing tests which was using shell expansion, since the glob was not quoted, and actually matching something unexpected (a folder). I will add a more in depth comment to the test. |
|
@brandonocasey The code looks clean! I'm on board with the this being a good breaking change. I don't completely understand this text from your comment:
Could you provide more detail? Thanks, again! |
|
Seems like my code comment got eaten by github. So I will give it another go. Basically the test that I fixed was unexpectedly using shell expansion since instead of Which causes an error because
Which means we only have one "pattern" so as long as it matches any files it will succeed. |
Proposed Changes
es-check should exit with an error if no files were checked. I would think this is either a breaking change, or that It will need to be behind an option. Personally though, I think that this should be on by default because reporting success when no files were check is not true.