-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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 3.5.0 #4032
Conversation
Note:
|
I'll rebase with the whitespace fix either tonight or early next week. Sorry! I was in a bit of a hurry to get this up before the weekend. |
Getting these errors:
This last error is different though. @othiym23 was this one added recently? I experienced this same issue with Kat and the 2.x PRs. This error is because node is built from source and is versioned as
|
@Fishrock123 those are the same test failures I was seeing, which aren't happening with 5.0.0 or Node 4 LTS. Like I said in the PR, we'll need to dig into this, probably next week, because I have no idea what's causing any of those failures and didn't have time to investigate. You, @zkat, and I should get on a hangout soon to talk about how to make the tests work better with |
@Fishrock123 Rebased, fixed, and generally cleaned up (the API documentation managed to sneak in yet again). We'll deal with the tests next week! |
Commits in
|
Copyright (c) npm, Inc. and Contributors | ||
All rights reserved. | ||
Licensed on the terms of The Artistic License 2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fishrock123
This is a much needed leveraging of Copyright...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Um, pardon? You mean All rights reserved.
needs to remain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fishrock123
I'll need to examine the full document but GH won't allow me to comment on lines that are too far out from the diff.
Basically npm should be able to define Terms of Service for npmjs.com and Terms of Use for the Software... spelling out exactly which is which and no "npm" (only) references. I know this directly from contractual obligations with closed-source agreements.
I have no difficultly with accepting a TOS for npmjs.com as it is their right to prevent bugged modules from being distributed being a pass through provider as long as it's clear in their TOS. With the FSF though as @scriptjs has pointed out the actual Code License hopefully continues to be FOSS. (Their TOU)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean All rights reserved. needs to remain?
No... that would not be leveraging the Copyright... which means node couldn't distribute it period as a module without a written agreement on paper registered with a legal authority... that costs $$ to get it registered and pretty much defeats the concept of FOSS.
@Fishrock123 |
@Fishrock I wanted to get to this today, but ran out of time first. I'll be digging into this first thing tomorrow. |
@othiym23 Sure thing. Just as a note I'll probably be busy from 12PM your time onwards though. |
@othiym23 If you have some instructions for running individual test files I can try to do a bisect. |
@Fishrock123 You can just run the individual tests with Node, and they all produce tap output. |
Looks like the tests fail on |
I'm waiting on npm 3.5.1 landing in master to re-test for AIX. From the earlier comments sounds like the plan may be merge that instead. @Fishrock123 is that correct ? |
@Fishrock123 is the process for merging/testing a new npm level documented somewhere ? I did a straight copy of 3.5.3 into a master checkout and a CI run looks clean (https://ci.nodejs.org/job/node-test-commit/1751/), but from the discussion I can tell you are also running the npm tests which I don't see a CI job for, and a straight copy might not be the way we merge. |
I'm waiting on npm > 3.5 landing in v5.x version |
I'm not really sure. @nodejs/npm could you open a new, updated PR we can look at for test failures? |
@Fishrock123 Sorry for the slow re-emergence from end of the year doldrums and lack of communication, but @iarna has been working on fixing npm's tests to work properly with prerelease versions of Node (and making npm just generally less noisy in that case). Expect to see a new PR containing those changes, but probably not until we get that version of npm released, in 10 days or so. |
@othiym23 Awesome, thanks for the update. |
👯 yay! thanks for this, will go well with our new canary-ish strategy coming soon |
This is being superseded by a PR for 3.6.0, which I'll be creating soon, as such I'm going to close this now. |
Thanks @iarna, if you can remember to run |
Sure will! =) Just waiting on a final test run to PR. |
Contains the changes in these releases:
Most notably, it includes the clarified split between Terms of Service and the CLI's own license (see the discussion in https://github.com/npm/npm/releases/tag/v3.5.1 and #3959 for more context). It also includes several important changes to how edge cases involving
bundleDependencies
are handled.Unfortunately, a few tests are failing when run against the Node build produced by current
master
, so some mixture of my, @iarna's, and probably @Fishrock123's time will be involved to sort out where those errors are coming from. Because there was some urgency expressed on the part of some folks on the TSC around the new licensing terms, I'm sending this PR anyway. We can pick up the discussion of the technical details next week, after the long US weekend.r: @Fishrock123
r: @jasnell
r: @mikeal