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

Fix Buffer deprecation warnings #552

Merged
merged 1 commit into from
Jan 17, 2019
Merged

Fix Buffer deprecation warnings #552

merged 1 commit into from
Jan 17, 2019

Conversation

ob-stripe
Copy link
Contributor

r? @remi-stripe @rattrayalex-stripe
cc @stripe/api-libraries

Using new Buffer is deprecated, cf. https://nodejs.org/api/buffer.html#buffer_new_buffer_array. This PR replaces uses of new Buffer with Buffer.alloc(size) or Buffer.from(string) as appropriate.

@remi-stripe
Copy link
Contributor

this LGTM but deferring to @rattrayalex-stripe who is more familiar with node

@remi-stripe remi-stripe removed their assignment Jan 17, 2019
@rattrayalex-stripe
Copy link
Contributor

Sadly it seems that both Buffer.from and Buffer.alloc were added in Node v5.10.0. It seems we claim support for Node 4.

I'd be in favor of supporting Node 6+ and dropping support for Node 4 and Node 5, which would of course require a major version bump. We could also check whether Buffer.from is defined before using it, etc.

@ob-stripe
Copy link
Contributor Author

@rattrayalex-stripe We use the safe-buffer package to backport those methods to early Node versions.

We should probably drop support for Node < 6 at some point, but we don't need to for this specific PR.

@rattrayalex-stripe
Copy link
Contributor

Ah, terrific! Sorry I missed that.

LGTM

@ob-stripe
Copy link
Contributor Author

No worries, thanks Alex!

@ob-stripe ob-stripe merged commit 57d5f09 into master Jan 17, 2019
@ob-stripe ob-stripe deleted the ob-fix-warning branch January 17, 2019 17:58
@ob-stripe
Copy link
Contributor Author

Released as 6.20.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants