-
Notifications
You must be signed in to change notification settings - Fork 101
Replace fs promises api with utils.promisify #67
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
Replace fs promises api with utils.promisify #67
Conversation
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.
Awesome, code changes LGTM! Just a quick nitpick 😄
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.
🚀 super!
|
For testing this change: to recap an offline conversation, exercising that file writing works could be done with a single |
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.
Oh shoot sorry @egiurleo I just noticed: https://github.com/egiurleo/tslint-to-eslint-config/blob/0620e729b8d9490f406659afd4ca54403721226e/src/adapters/fsFileSystem.ts#L25
That usage of fs.promises will need to be replaced too. Could you please make that change?
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.
Wonderful, thanks so much for sending this in @egiurleo! 🥇
PR Checklist
status: accepting prsOverview
This PR replaces the
fs.promisesAPI withutil.promisifyin order to support Node 8.x.Edit: Not sure how to test this change.