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.3 #1219

Closed
wants to merge 2 commits into from
Closed

deps: upgrade npm to 2.7.3 #1219

wants to merge 2 commits into from

Conversation

othiym23
Copy link
Contributor

Since I'm now an io.js collaborator, I'm happy to merge this myself. However, I still need sign-off from one or more other collaborators, so give this a look if you have a few minutes.

This is another bug release, and this is actually a roll-up of two versions, because an interim release was made to the canary release to fix a nasty regression that had crept in:

  • 1549106
    #7641 Due to 448efd0, running npm shrinkwrap --dev caused production dependencies to no longer be included in
    npm-shrinkwrap.json. Whoopsie! (@othiym23)

Aside from that, here are the two most important fixes from 2.7.2:

  • fb0ac26
    #7579 Only block removing files and
    links when we're sure npm isn't responsible for them. This change is hard to
    summarize, because if things are working correctly you should never see it,
    but if you want more context, just go read the commit
    message
    ,
    which lays it all out. (@othiym23)
  • 051c473
    #7552 bundledDependencies are now
    properly included in the installation context. This is another fantastically
    hard-to-summarize bug, and once again, I encourage you to read the commit
    message

    if you're curious about the details. The snappy takeaway is that this
    unbreaks many use cases for ember-cli. (@othiym23)

There are, as usual, a few other smaller fixes in the release as well. I've applied the floating patch for node-gyp yet again, and would very much like to not have to do this for too many more releases. Please?

R=@rvagg
R=@Fishrock123

@rvagg
Copy link
Member

rvagg commented Mar 20, 2015

LGTM

I know of at least one large company that's shipping off npm shrinkwrap --dev files to nsp, so they might appreciate getting prod deps in there!

And ,, what's going on with your AUTHORS file?

@kenany
Copy link
Contributor

kenany commented Mar 20, 2015

@rvagg We discovered just as you did that awk on OS X is bad at keeping the sort order. Now we use perl npm/npm#7597 :)

@Fishrock123
Copy link
Contributor

@othiym23 Can you make sure to use --whitespace=fix when merging? There's some whitespace errors that always crop up in these. :)

deps/npm/node_modules/npm-registry-client/test/fixtures/underscore/1.3.3/package.tgz

ehhh

make test-npm is happy. Reviewed code. LGTM otherwise.

Edit: Also make test-npm leaves a bunch of garbage lying around after, not sure if that is on our end or npm's end?

@rvagg
Copy link
Member

rvagg commented Mar 23, 2015

@othiym23 if you land this within the next couple of hours we can include it in 1.6.2.

@Fishrock123
Copy link
Contributor

If @othiym23 isn't around, I move we just land it as we normally have.

@bricss
Copy link

bricss commented Mar 23, 2015

npm v2.7.4 already arrived.

@kenany
Copy link
Contributor

kenany commented Mar 23, 2015

@bricss But it's a pre-release so don't bring that up until next Thursday :) https://github.com/npm/npm/wiki/Release-Process

othiym23 and others added 2 commits March 23, 2015 15:30
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>
othiym23 added a commit that referenced this pull request Mar 23, 2015
PR-URL: #1219
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@othiym23
Copy link
Contributor Author

Landed in b542fb9 with the rollup node-gyp patch in 269e46b. Thanks to everyone for their review and helping me figure out how to get this landed!

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.

6 participants