Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

deps: pin request to ~2.81.0 #1300

Closed
wants to merge 0 commits into from

Conversation

refack
Copy link
Contributor

@refack refack commented Sep 26, 2017

Checklist
Description of change

Required for "node < 4" compatibility and is congruent with npm

Fixes: #1299

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM, although it's going to diverge (and more importantly: cause duplication) when npm upgrades to request 2.9.0.

@bnoordhuis
Copy link
Member

Another thing, if we ever decide to upgrade the node-gyp in the v4.x and v6.x branches, that too would result in duplication: their copies of npm are stuck on request@2.75.0.

Not the end of the world, just something to keep in mind.

@refack
Copy link
Contributor Author

refack commented Sep 27, 2017

Another thing, if we ever decide to upgrade the node-gyp in the v4.x and v6.x branches, that too would result in duplication: their copies of npm are stuck on request@2.75.0.

I'll test if <2.82.0 get npm to dedup...

@refack
Copy link
Contributor Author

refack commented Sep 27, 2017

Indeed setting to <2.82.0 allows deduping:

$ npm ls | grep req
npm@2.15.12 d:\code\4node\test-npm-request\npm
| +-- request@2.74.0 deduped
| +-- request@2.74.0 deduped
+-- request@2.74.0

@addaleax
Copy link
Member

@refack can you also give the minimum version in that specifier if you use <? I think right now you’d match any version of request before 2.82.0, up from 0.8.3 (which I assume does not work :))

@refack
Copy link
Contributor Author

refack commented Sep 27, 2017

I think right now you’d match any version of request before 2.82.0, up from 0.8.3 (which I assume does not work :))

Ack.
Did some bisecting - minimal version to pass npm test is 2.9.0.

@addaleax
Copy link
Member

thanks!

@bnoordhuis
Copy link
Member

Ping @refack.

refack added a commit that referenced this pull request Oct 10, 2017
Required for "node < 4" compatibility and is congruent with `npm`

minimum for passing `npm test` >= 2.9.0
setting < 2.82.0 allows deduping

PR-URL: #1300
Fixes: #1299
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@refack refack closed this Oct 10, 2017
@refack refack force-pushed the ping-request-on3-branch branch from a60bd45 to 7900122 Compare October 10, 2017 23:22
@edi9999
Copy link

edi9999 commented Jan 15, 2018

@refack Could you please release of the 3.x with this change ?

@refack refack deleted the ping-request-on3-branch branch June 14, 2018 14:34
rvagg pushed a commit that referenced this pull request Aug 9, 2018
Required for "node < 4" compatibility and is congruent with `npm`

minimum for passing `npm test` >= 2.9.0
setting < 2.82.0 allows deduping

PR-URL: #1300
Fixes: #1299
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
rvagg pushed a commit that referenced this pull request Apr 24, 2019
Required for "node < 4" compatibility and is congruent with `npm`

minimum for passing `npm test` >= 2.9.0
setting < 2.82.0 allows deduping

PR-URL: #1300
Fixes: #1299
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@rvagg rvagg mentioned this pull request Apr 24, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants