Skip to content

use new to instantiate all buffers #75

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

Merged
merged 1 commit into from
Nov 2, 2016

Conversation

MylesBorins
Copy link
Contributor

Using Buffer without new will soon stop working. It will throw
A deprecation warning in Node v7.

Using Buffer without `new` will soon stop working. It will throw
A deprecation warning in Node v7.
MylesBorins pushed a commit to MylesBorins/node-browserify that referenced this pull request Aug 31, 2016
Using Buffer without new will soon stop working. It will throw
A deprecation warning in Node v7. This commit modifies the tests
in `test/bare.js` and `test/bare_shebang.js` to use the
`Buffer.from` interface instead. It is a drop in replacement for
Buffer that is supported by Node v4 / v6 and Browserify's Buffer
implementation

For the browserify test suite to pass on Node v7 we will need
to land this commit as well as the two below PR's

* browserify/browser-pack#75
* max-mapper/concat-stream#48
@TehShrike
Copy link

lgtm

@TehShrike
Copy link

btw, this pull request fixes #76.

@oleavr
Copy link

oleavr commented Nov 2, 2016

Bump

@ForbesLindesay
Copy link
Collaborator

I can't see any problem with this.

@ForbesLindesay ForbesLindesay merged commit c91b816 into browserify:master Nov 2, 2016
@danez
Copy link

danez commented Nov 9, 2016

Can we please get a bugfix release for this? That would be nice.

@ghost
Copy link

ghost commented Nov 22, 2016

This doesn't fix anything. There is no clear roadmap for core. Adding new does literally nothing except removes a warning on an already-deprecated API. Avoid doing anything until core gets its shit together.

@TehShrike
Copy link

It fixes issue #76, "annoying warning showing up in logs".

@danez
Copy link

danez commented Nov 22, 2016

Also don't see what the problem is with releasing a patch version in this case. Yes no logic changed, but having no deprecation warnings is a good thing.

/cc @zertosh

@ghost
Copy link

ghost commented Feb 18, 2017

This shouldn't have been merged but I don't blame anyone except for node core.

@MylesBorins
Copy link
Contributor Author

MylesBorins commented Feb 19, 2017

@substack we reverted in core. LMK if you would like me to submit a revert PR to browser-pack

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.

5 participants