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

Use Buffer.from() instead of new Buffer #2

Merged
merged 1 commit into from
Jan 20, 2020

Conversation

addaleax
Copy link
Contributor

The Buffer constructor is deprecated, we should
use Buffer.from() instead.

Refs: nodejs/node#19079

The `Buffer` constructor is deprecated, we should
use `Buffer.from()` instead.

Refs: nodejs/node#19079
@Trott
Copy link
Contributor

Trott commented Oct 10, 2019

Polite bump! The engines field in package.json indicates that this package is for Node.js 8.x and newer. If that's a correct conclusion, then this should be safe to land.

@ruyadorno
Copy link
Contributor

The engines field in package.json indicates that this package is for Node.js 8.x and newer. If that's a correct conclusion, then this should be safe to land.

@Trott in that case this change would land only in npm@7 - the current npm@6.x line is still on fs-minipass@1.2.7 and that should support node@6

@Trott
Copy link
Contributor

Trott commented Jan 14, 2020

The engines field in package.json indicates that this package is for Node.js 8.x and newer. If that's a correct conclusion, then this should be safe to land.

@Trott in that case this change would land only in npm@7 - the current npm@6.x line is still on fs-minipass@1.2.7 and that should support node@6

I see. Thanks!

FWIW, Buffer.from(string,encoding) was added in Node.js 5.x, so this PR should be good to support Node.js 6.x. But I guess it's probably not worth backporting to a release branch. Still, having it in a (I guess semver major) release of fs-minipass would allow some real-world testing before it ends up shipping in npm itself.

@ruyadorno ruyadorno merged commit 8b16337 into isaacs:master Jan 20, 2020
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