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.

Sign up for GitHub

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

Jump to bottom

Avoid using deprecated Buffer constructor #25

Merged
merged 2 commits into from
Dec 12, 2018
Merged

Avoid using deprecated Buffer constructor #25

merged 2 commits into from
Dec 12, 2018

Conversation

ChALkeR
Copy link
Contributor

@ChALkeR ChALkeR commented Feb 23, 2018 •
edited
Loading

This raises the minimum Node.js version from 4.3.0 to 4.5.0, but that should be fine, as 4.5.0 was released very long ago and even the whole 4.x branch will be out of LTS support soon.

Probably almost no one uses Node.js versions 4.3.x and 4.4.x nowdays.

webpack/webpack states "node": ">=6.11.5" requirement already.

Also this removes those // eslint-disable-line comments. 😉

Lint failure is unrelated and happens because of the function named async, upon fixing that — tests pass locally.

Refs:
https://nodejs.org/api/deprecations.html#deprecations_dep0005_buffer_constructor
webpack/webpack#6356

@ChALkeR
Avoid using deprecated Buffer constructor
25a2980 
This raises the minimum Node.js version from 4.3.0 to 4.5.0,
but that should be fine, as 4.5.0 was released very long ago
and even the whole 4.x branch will be out of LTS support soon.

Probably almost no one uses Node.js versions 4.3.x and 4.4.x nowdays.

webpack/webpack states node": ">=6.11.5" requirement already.

Refs:
https://nodejs.org/api/deprecations.html#deprecations_dep0005_buffer_constructor
webpack/webpack#6356
@jsf-clabot
Copy link

jsf-clabot commented Feb 23, 2018 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@ChALkeR
Copy link
Contributor Author

ChALkeR commented Mar 2, 2018

Tracking: nodejs/node#19079

ooflorent
ooflorent approved these changes Mar 2, 2018
View reviewed changes
Copy link
Member

@ooflorent ooflorent left a comment

Choose a reason for hiding this comment

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

Thanks!

@ChALkeR
Copy link
Contributor Author

ChALkeR commented Mar 2, 2018

Travis apparently fails because it tests on Node.js 4.0 and 5.0, instead of 4.x (or 4.5) and 5.x (or 5.10).

@ChALkeR
Copy link
Contributor Author

ChALkeR commented May 18, 2018

/cc @TheLarkInn

@ChALkeR
Copy link
Contributor Author

ChALkeR commented Jul 8, 2018 •
edited
Loading

Any updates?
#26 fixes Travis targets.

@ChALkeR
Copy link
Contributor Author

ChALkeR commented Jul 20, 2018 •
edited
Loading

@ooflorent @TheLarkInn @sokra This constantly bumps in my recent activitity feed due to @jsf-clabot updating the status several times per day. Could you please disable that?

screenshot_20180720_193255

«jsf-clabot edited on 26 Mar (most recent)» is incorrect, the history is truncated and shows the first 100 entries out of 341 total atm — it gets bumped every day, several times per day.

@ChALkeR
Copy link
Contributor Author

ChALkeR commented Jul 22, 2018

screenshot_20180722_144938

@ChALkeR
Copy link
Contributor Author

ChALkeR commented Jul 22, 2018

Second time today:
screenshot_20180722_233006

@ChALkeR ChALkeR mentioned this pull request Jul 23, 2018
Open
@ChALkeR
Copy link
Contributor Author

ChALkeR commented Jul 24, 2018 •
edited
Loading

Unsubscribing didn't help.

screenshot_20180724_061333

I'm going to close this, feel free to reopen if needed (or when the CLA bot gets fixed).

@ChALkeR ChALkeR closed this Jul 24, 2018
@ChALkeR
Copy link
Contributor Author

ChALkeR commented Aug 8, 2018

Let's try reopening this.

@ChALkeR ChALkeR reopened this Aug 8, 2018
@ChALkeR
Copy link
Contributor Author

ChALkeR commented Aug 14, 2018

Keeps happening, last time was 6 hours ago.
@JSFOwner please reopen this once the issue with CLA bot will be fixed.

@ChALkeR ChALkeR closed this Aug 14, 2018
@ChALkeR ChALkeR mentioned this pull request Dec 7, 2018
Merged
@noscripter
Copy link
Contributor

Please merge this pr.

@sokra sokra reopened this Dec 12, 2018
@sokra sokra closed this Dec 12, 2018
@sokra sokra reopened this Dec 12, 2018
@codecov
Copy link

codecov bot commented Dec 12, 2018 •
edited
Loading

Codecov Report

Merging #25 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #25      +/-   ##
==========================================
+ Coverage    91.3%   91.37%   +0.06%     
==========================================
  Files           2        2              
  Lines         253      255       +2     
  Branches       67       67              
==========================================
+ Hits          231      233       +2     
  Misses         22       22
Impacted Files Coverage Δ
lib/LoaderRunner.js 94.34% <100%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d41655...7f0df0d. Read the comment docs.

@sokra sokra merged commit ca1c100 into webpack:master Dec 12, 2018
@sokra
Copy link
Member

sokra commented Dec 12, 2018

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ooflorent ooflorent ooflorent approved these changes

@sokra sokra sokra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ChALkeR @jsf-clabot @noscripter @sokra @ooflorent