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

deps: upgrade npm to 4.1.2 #11020

Closed
wants to merge 1 commit into from
Closed

deps: upgrade npm to 4.1.2 #11020

wants to merge 1 commit into from

Conversation

zkat
Copy link
Contributor

@zkat zkat commented Jan 26, 2017 •
edited
Loading

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
  • deps
Description of Change
Additional notes

Heads-up that npm@next is currently a semver-minor bump to npm@4.2.0 which includes a significant improvement for search. We'll downstream it in two weeks but it might be worth pointing out that bump cause it's something folks might want in LTS (specially since it has to do with a side utility/service rather than installation and the like). Cheers!

Changelogs

r: @Fishrock123
r: @jasnell
r: @addaleax

@nodejs-github-bot nodejs-github-bot added the npm Issues and PRs related to the npm client dependency or the npm registry. label Jan 26, 2017
@jasnell
Copy link
Member

jasnell commented Jan 26, 2017

@zkat ... I appreciate this. It would be helpful if you would consider adding links to the relevant commits/PRs for each of the "description of change" bullet points. In particular I'd like to review the file URL changes a bit more in detail to understand more about what is happening there. I will try to give this a review by early next week

@Fishrock123
Copy link
Contributor

@zkat oh no.

Our old friend is back... in a new form:

Applying: deps: upgrade npm to 4.1.2
error: deps/npm/node_modules/request/node_modules/form-data/README.md: already exists in working directory
Patch failed at 0001 deps: upgrade npm to 4.1.2

@Fishrock123
Copy link
Contributor

@zkat In the mean time I'm going to merge #10781, can you rebase after and then we can try to fix that? :D

@Fishrock123
Copy link
Contributor

Ok, 4.1.1 merged. I guess the best way forward here is to rename the file in a separate commit first and then do the rest including whatever changed within the file?

@zkat
Copy link
Contributor Author

zkat commented Jan 26, 2017

@Fishrock123 I'm so angry. I'm so so angry. I hate git so much sometimes. I hate OSX's (and Windows') wobbly bullshit with case-insensitivity. I hate joy and happyness.... anyway yeah that sounds like a good plan. I'll try it again in a wee bit.

@jasnell Yeah, one sec and I'll look those up for you. Watch for an edit in a few minutes.

@zkat
Copy link
Contributor Author

zkat commented Jan 26, 2017

@Fishrock123 I rebased and did --whitespace=fix again on mine and ran into no issues. You should definitely be able to merge through the GH UI now, but you may have to do some local maneuvering if you're still running into this shit locally. I'm sorry. I've tried. 😭

@zkat
Copy link
Contributor Author

zkat commented Jan 26, 2017

@jasnell incidentally, the url.format patch: the only change made was literally just that we stopped using it in one tiny case where we really didn't have to, so that's all it took for it to stop affecting us. I don't think it'll need any further changes. This way is safer anyway. But the link is in the edit now. :)

@Fishrock123
Copy link
Contributor

@zkat definitely still running into issues. :/

@zkat
Copy link
Contributor Author

zkat commented Jan 26, 2017

@Fishrock123 one more try? Just pushed a new one.

@Fishrock123
Copy link
Contributor

@zkat Nice, great work. Seems to land fine. Here's to hoping it doesn't also have lingering side effects.

Curious though, what did you do?

@zkat
Copy link
Contributor Author

zkat commented Jan 26, 2017

@Fishrock123

git reset head~1
cd deps/npm/(...)/form-data
# need to do a two-step move because OSX will noop a plain case-only move
mv README.md rm.md
mv rm.md Readme.md
git add -A && git commit -m 'deps: upgrade npm to 4.1.2'

And then I just check the commit summary to make sure there's not a file addition for README.md.

@targos
Copy link
Member

targos commented Jan 26, 2017

Changes from all commits 1,035 files +919 −21,259

That is cool!

@zkat
Copy link
Contributor Author

zkat commented Jan 26, 2017

@targos not much to get excited about, imo. that's probably just from some part of the dependency tree being flattened because of recursive dependencies getting updated and thus allowing more deduping.

@targos
Copy link
Member

targos commented Jan 26, 2017

Not much I agree, but the tarball should still be noticeably smaller

@zkat
Copy link
Contributor Author

zkat commented Jan 26, 2017

@targos absolutely! you're right. And that's actually p. nice.

@Fishrock123 Fishrock123 self-assigned this Jan 26, 2017
Fishrock123
Fishrock123 approved these changes Jan 26, 2017
View reviewed changes
Copy link
Contributor

@Fishrock123 Fishrock123 left a comment

Choose a reason for hiding this comment

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

Test pass, LGTM

@jasnell
Copy link
Member

jasnell commented Jan 27, 2017

thank you @zkat!

@bricss bricss mentioned this pull request Jan 29, 2017
Merged
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 30, 2017
@zkat @Fishrock123
deps: upgrade npm to 4.1.2
afb7c1b 
PR-URL: nodejs#11020
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@Fishrock123
Copy link
Contributor

Thanks, landed in afb7c1b!

@joaocgreis
Copy link
Member

Was there a CI run on this PR? Anyone still has the link?

@joaocgreis joaocgreis mentioned this pull request Jan 31, 2017
Closed
2 tasks
@MylesBorins
Copy link
Contributor

when this lands it will need to come with 35e749b

evanlucas pushed a commit that referenced this pull request Jan 31, 2017
@zkat @evanlucas
deps: upgrade npm to 4.1.2
9e60af8 
PR-URL: #11020
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
evanlucas added a commit that referenced this pull request Jan 31, 2017
@evanlucas
2017-01-31, Version 7.5.0 (Current)
a34f1d6 
Notable changes:

* crypto:
  * ability to select cert store at runtime (Adam Majer) #8334
  * Use system CAs instead of using bundled ones (Adam Majer) #8334
* deps:
  * upgrade npm to 4.1.2 (Kat Marchán) #11020
  * upgrade openssl sources to 1.0.2k (Shigeki Ohtsu) #11021
* doc: add basic documentation for WHATWG URL API (James M Snell) #10620
* process: add NODE_NO_WARNINGS environment variable (cjihrig) #10842
* url: allow use of URL with http.request and https.request (James M Snell) #10638

PR-URL: #11062
evanlucas added a commit that referenced this pull request Feb 1, 2017
@evanlucas
2017-01-31, Version 7.5.0 (Current)
a1c91ec 
Notable changes:

* crypto:
  * ability to select cert store at runtime (Adam Majer) #8334
  * Use system CAs instead of using bundled ones (Adam Majer) #8334
* deps:
  * upgrade npm to 4.1.2 (Kat Marchán) #11020
  * upgrade openssl sources to 1.0.2k (Shigeki Ohtsu) #11021
* doc: add basic documentation for WHATWG URL API (James M Snell) #10620
* process: add NODE_NO_WARNINGS environment variable (cjihrig) #10842
* url: allow use of URL with http.request and https.request (James M Snell) #10638

PR-URL: #11062
@joshgav joshgav mentioned this pull request Feb 13, 2017
Closed
3 tasks
@Fishrock123 Fishrock123 mentioned this pull request Apr 24, 2017
Closed
Fishrock123 added a commit that referenced this pull request Apr 26, 2017
@Fishrock123
deps: remove **/node_modules/form-data/README.md
e8f3163 
Having multiple files with the same name but different casings causes
problems on lots of OS-s.

Refs: #12624
Refs: #11085
Refs: #11020

PR-URL: #12643
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
evanlucas pushed a commit that referenced this pull request May 1, 2017
@Fishrock123 @evanlucas
deps: remove **/node_modules/form-data/README.md
41e052a 
Having multiple files with the same name but different casings causes
problems on lots of OS-s.

Refs: #12624
Refs: #11085
Refs: #11020

PR-URL: #12643
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
evanlucas pushed a commit that referenced this pull request May 2, 2017
@Fishrock123 @evanlucas
deps: remove **/node_modules/form-data/README.md
f8ae23e 
Having multiple files with the same name but different casings causes
problems on lots of OS-s.

Refs: #12624
Refs: #11085
Refs: #11020

PR-URL: #12643
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
evanlucas pushed a commit that referenced this pull request May 2, 2017
@Fishrock123 @evanlucas
deps: remove **/node_modules/form-data/README.md
d154aaf 
Having multiple files with the same name but different casings causes
problems on lots of OS-s.

Refs: #12624
Refs: #11085
Refs: #11020

PR-URL: #12643
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fishrock123 Fishrock123 Fishrock123 approved these changes

Assignees

@Fishrock123 Fishrock123

Labels
npm Issues and PRs related to the npm client dependency or the npm registry.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@zkat @jasnell @Fishrock123 @targos @joaocgreis @MylesBorins @mscdex @nodejs-github-bot