-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Fix: use async reading of stdin in bin/eslint.js #12230
Fix: use async reading of stdin in bin/eslint.js #12230
Conversation
Thank you for your contribution. Would you add a test for this? |
I can, it'll take me a day or so to clear my work to find the time for this. I'll have a look in the test cases, but all I need is a test that uses a large file (anything over 4k in size). Does anyone know which test I can look at off the top of their heads? |
ed18fc9
to
7c2387e
Compare
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.
LGTM, thanks!
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.
LGTM, thank you.
Ah, @jsf-clabot seems to report a problem. Would you check the e-mail address of your commits? |
@mysticatea that is because I pushed my changes from my work machine with a work email. I then fixed that to my gmail account and force pushed to remove that work email. Its now saying all checks passed, including licece/cla. |
The bot comment has been fixed 5 hours ago. |
closes #12212
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ X] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
See #12212
What changes did you make? (Give an overview)
Applied nodejs recommendations for processing stdin as per https://github.com/nodejs/node/blob/master/doc/api/process.md#processstdin
Is there anything you'd like reviewers to focus on?
Compatibility with other operating systems.
I've only tested this on Windows.