Skip to content
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

Node 8.1.3 #306

Merged
merged 7 commits into from
Jun 29, 2017
Merged

Node 8.1.3 #306

merged 7 commits into from
Jun 29, 2017

Conversation

mcollina
Copy link
Member

Includes #304 and #303.

Fixes #302
Fixes #305

function _uint8ArrayToBuffer(chunk) {
return Buffer.from(chunk);
}
function _isUint8Array(obj) {
return Object.prototype.toString.call(obj) === '[object Uint8Array]' || Buffer.isBuffer(obj);
return Buffer.isBuffer(obj) || obj instanceof OurUint8Array;

Choose a reason for hiding this comment

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

Just to check: this is meant to always pass in environments that don't have global.Uint8Array, right?

Choose a reason for hiding this comment

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

e.g. resolves to obj instanceOf Function

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes exactly.

@@ -660,4 +661,4 @@ Writable.prototype._undestroy = destroyImpl.undestroy;
Writable.prototype._destroy = function (err, cb) {
this.end();
cb(err);
};
};

Choose a reason for hiding this comment

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

lil nit

}));

_stream2.write(undefined);
}

Choose a reason for hiding this comment

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

lil nit

Copy link

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

not seeing anything in particular that needs changing; few comments but those are optional - LGTM

Copy link
Contributor

@lrlna lrlna left a comment

Choose a reason for hiding this comment

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

ace 💯

@mcollina
Copy link
Member Author

The nits are out of babel, so there is not much we can do about them

@mcollina mcollina merged commit d6c391d into master Jun 29, 2017
@mcollina mcollina deleted the node-8-1-3 branch June 29, 2017 14:16
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.

4 participants