Skip to content

Conversation

@soyuka
Copy link
Contributor

@soyuka soyuka commented Jun 17, 2017

This is a breaking change.

this._finalize();
}

return this;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return signature changes hence backward compatibility break

return new Promise(function(resolve, reject) {
var errored;

self._module.on('end', function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lmk if close is better here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum, that difference usually matters more for fs.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return this;
var self = this:

return new Promise(function(resolve, reject) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of self we could use an arrow function (note sure node 4 supports it without the --harmony flag)

@soyuka soyuka force-pushed the promise-support branch from bce192d to 35d86a5 Compare June 17, 2017 01:28
@soyuka soyuka force-pushed the promise-support branch from 35d86a5 to d1b4478 Compare June 17, 2017 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants