Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(377)

Issue 8714043: Freeze the versions of all nodejs dependencies.

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 7 months ago by teknico
Modified:
8 years, 7 months ago
Reviewers:
mp+158659, frankban, gary.poster
Visibility:
Public.

Description

Freeze the versions of all nodejs dependencies. Add a generated npm-shrinkwrap.json file that freezes the current versions of all nodejs dependecies, even the recursive ones. Update the package.json file keeping the currently fixed package versions. Also remove the misleading "engines" entry (see <https://npmjs.org/doc/json.html#engines>), and remove the useless distinction between "dependencies" and "devDependencies" (see <https://npmjs.org/doc/json.html#devDependencies>). Add a documentation section, "Updating the nodejs dependencies" at the bottom of the docs/hacking.rst file, explaining how this works and how to update dependencies to new versions. Please check that the whole thing is understandable and workable: you will use this stuff, after all. :-) Sorry for the change size: it's mainly due to the 1k+ lines of the generated npm-shrinkwrap.json file. No need to go over that one, just look at the changes in the docs/hacking.rst and package.json files. https://code.launchpad.net/~teknico/juju-gui/harden-npm-dependencies/+merge/158659 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 4

Patch Set 2 : Freeze the versions of all nodejs dependencies. #

Total comments: 4

Patch Set 3 : Freeze the versions of all nodejs dependencies. #

Patch Set 4 : Freeze the versions of all nodejs dependencies. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1087 lines, -23 lines) Patch
M HACKING View 1 2 5 chunks +45 lines, -9 lines 0 comments Download
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A npm-shrinkwrap.json View 1 chunk +1032 lines, -0 lines 0 comments Download
M package.json View 1 2 chunks +8 lines, -14 lines 0 comments Download

Messages

Total messages: 7
teknico
Please take a look.
8 years, 7 months ago (2013-04-12 15:57:53 UTC) #1
gary.poster
LGTM. I tried this in the charm (PyJuju) and it worked great. Thank you! Gary ...
8 years, 7 months ago (2013-04-12 22:02:20 UTC) #2
teknico
Thanks for the review. No remarks on the docs? https://codereview.appspot.com/8714043/diff/1/package.json File package.json (left): https://codereview.appspot.com/8714043/diff/1/package.json#oldcode17 package.json:17: ...
8 years, 7 months ago (2013-04-15 10:09:08 UTC) #3
teknico
Please take a look.
8 years, 7 months ago (2013-04-15 10:15:14 UTC) #4
frankban
LGTM, see just a minor below. Thank you. https://codereview.appspot.com/8714043/diff/5001/HACKING File HACKING (right): https://codereview.appspot.com/8714043/diff/5001/HACKING#newcode57 HACKING:57: sudo ...
8 years, 7 months ago (2013-04-15 10:44:02 UTC) #5
teknico
https://codereview.appspot.com/8714043/diff/5001/HACKING File HACKING (right): https://codereview.appspot.com/8714043/diff/5001/HACKING#newcode57 HACKING:57: sudo apt-get install nodejs imagemagick lbox python-sphinx python-yaml python-tz ...
8 years, 7 months ago (2013-04-15 10:49:21 UTC) #6
teknico
8 years, 7 months ago (2013-04-15 11:24:29 UTC) #7
*** Submitted:

Freeze the versions of all nodejs dependencies.

Add a generated npm-shrinkwrap.json file that freezes the current versions
of all nodejs dependecies, even the recursive ones.

Update the package.json file keeping the currently fixed package versions.
Also remove the misleading "engines" entry (see
<https://npmjs.org/doc/json.html#engines>), and remove the useless distinction
between "dependencies" and "devDependencies" (see
<https://npmjs.org/doc/json.html#devDependencies>).

Add a documentation section, "Updating the nodejs dependencies" at the
bottom of the docs/hacking.rst file, explaining how this works and how to
update dependencies to new versions. Please check that the whole thing is
understandable and workable: you will use this stuff, after all. :-)

Sorry for the change size: it's mainly due to the 1k+ lines of the generated
npm-shrinkwrap.json file. No need to go over that one, just look at the
changes in the docs/hacking.rst and package.json files.

R=gary.poster, frankban
CC=
https://codereview.appspot.com/8714043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b