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

deps: upgrade npm to 2.7.1 #1142

Closed
wants to merge 2 commits into from
Closed

deps: upgrade npm to 2.7.1 #1142

wants to merge 2 commits into from

Conversation

othiym23
Copy link
Contributor

npm@2.7.1 is a pretty hefty release. I've included the four most significant changes below, and there are many others in the full release notes.

  • 6823807
    #7121 npm install --save for Git
    dependencies saves the URL passed in, instead of the temporary directory used
    to clone the remote repo. Fixes using Git dependencies when shrinkwwapping.
    In the process, rewrote the Git dependency caching code. Again. No more
    single-letter variable names, and a much clearer workflow.
    (@othiym23)
  • abdd040
    read-package-json@1.3.2: Provide more helpful error messages when JSON
    parse errors are encountered by using a more forgiving JSON parser than
    JSON.parse. (@smikes)
  • c56cfcd
    #7525 npm dedupe handles scoped
    packages. (@KidkArolis)
  • 4ef1412
    #7075 If you try to tag a release
    as a valid semver range, npm publish and npm tag will error early instead
    of proceeding. (@smikes)

I've also applied the floating patch to node-gyp to make it work properly with io.js

othiym23 and others added 2 commits March 13, 2015 02:07
Every npm version bump requires a few patches to be floated on
node-gyp for io.js compatibility. These patches are found in
03d1992,
5de334c, and
da730c7. This commit squashes
them into a single commit.

PR-URL: nodejs#990
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@Fishrock123
Copy link
Contributor

make test and make test-npm are fine for me locally. Skimmed over the code, LGTM.

(I pinged @rvagg on twitter, maybe we can get a npm-specific CI test to make reviewing these more reliable multiplatform?)

@Fishrock123
Copy link
Contributor

My bad, looks like there is a CI task for it now!

https://jenkins-iojs.nodesource.com/job/iojs+npm+any-pr+multi/2/

Edit: looks like the node symlink may not be working on some CI platforms?

@rvagg
Copy link
Member

rvagg commented Mar 13, 2015

@Fishrock123 to expand on the earlier conversation: the test-npm Jenkins job initially exists to give @othiym23 a place to refine his test suite to make it work for us, don't expect it to actually work yet hence the sh: node: not found failures. Note also though I don't think I've actually given @othiym23 his Jenkins password yet! I've been so bogged down with work these past 3 weeks that lots of things have been pushed to the side but I expect to clear some of my OSS backlog next week when I get some time off. I have Jenkins credentials for all collaborators to hand out but I need to finish my doc about using Jenkins (in PR somewhere in iojs/build)

This PR gets an LGTM from me.

@rvagg rvagg mentioned this pull request Mar 14, 2015
Fishrock123 pushed a commit that referenced this pull request Mar 16, 2015
PR-URL: #1142
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@Fishrock123
Copy link
Contributor

Thanks, landed in 7d0baf1 and 65d0a8e :shipit:

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.

4 participants