Skip to content

use new to instantiate all buffers - #75

Merged
ForbesLindesay merged 1 commit into
browserify:masterfrom
MylesBorins:new-Buffer
Nov 2, 2016
Merged

ForbesLindesay merged 1 commit into
browserify:masterfrom
MylesBorins:new-Buffer

Conversation

@MylesBorins

Copy link
Copy Markdown
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
Copy Markdown

lgtm

@TehShrike

Copy link
Copy Markdown

btw, this pull request fixes #76.

@oleavr

oleavr commented Nov 2, 2016

Copy link
Copy Markdown

Bump

@ForbesLindesay

Copy link
Copy Markdown
Collaborator

I can't see any problem with this.

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

danez commented Nov 9, 2016

Copy link
Copy Markdown

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

@ghost

ghost commented Nov 22, 2016 •

Copy link
Copy Markdown

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
Copy Markdown

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

@danez

danez commented Nov 22, 2016

Copy link
Copy Markdown

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

ghost commented Feb 18, 2017 •

Copy link
Copy Markdown

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

@MylesBorins

MylesBorins commented Feb 19, 2017 •

Copy link
Copy Markdown
Contributor Author

@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