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

Issue 29526588: Issue 5559 - include Node.js in ensure_dependencies.py (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 7 months ago by tlucas
Modified:
2 years, 7 months ago
CC:
kzar
Visibility:
Public.

Description

Issue 5559 - include Node.js in ensure_dependencies.py

Patch Set 1 #

Total comments: 27

Patch Set 2 : Refactoring according to new integration notes #

Total comments: 2

Patch Set 3 : #

Total comments: 6

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -5 lines) Patch
M ensure_dependencies.py View 1 2 3 6 chunks +49 lines, -5 lines 0 comments Download

Messages

Total messages: 13
tlucas
Patch Set 1 Let ensure_dependencies.py install Node.js dependencies.
2 years, 7 months ago (2017-08-24 11:06:23 UTC) #1
Wladimir Palant
https://codereview.adblockplus.org/29526588/diff/29526589/ensure_dependencies.py File ensure_dependencies.py (right): https://codereview.adblockplus.org/29526588/diff/29526589/ensure_dependencies.py#newcode258 ensure_dependencies.py:258: Do nothing when the SKIP_DEPENDENCY_UPDATES environment variable is set. ...
2 years, 7 months ago (2017-08-24 11:38:13 UTC) #2
tlucas
https://codereview.adblockplus.org/29526588/diff/29526589/ensure_dependencies.py File ensure_dependencies.py (right): https://codereview.adblockplus.org/29526588/diff/29526589/ensure_dependencies.py#newcode258 ensure_dependencies.py:258: Do nothing when the SKIP_DEPENDENCY_UPDATES environment variable is set. ...
2 years, 7 months ago (2017-08-24 13:27:55 UTC) #3
Wladimir Palant
https://codereview.adblockplus.org/29526588/diff/29526589/ensure_dependencies.py File ensure_dependencies.py (right): https://codereview.adblockplus.org/29526588/diff/29526589/ensure_dependencies.py#newcode281 ensure_dependencies.py:281: if subprocess.call(['npm', 'outdated'], **subprocess_kwargs) == 0: On 2017/08/24 13:27:54, ...
2 years, 7 months ago (2017-08-25 08:10:42 UTC) #4
Wladimir Palant
https://codereview.adblockplus.org/29526588/diff/29526589/ensure_dependencies.py File ensure_dependencies.py (right): https://codereview.adblockplus.org/29526588/diff/29526589/ensure_dependencies.py#newcode281 ensure_dependencies.py:281: if subprocess.call(['npm', 'outdated'], **subprocess_kwargs) == 0: On 2017/08/25 08:10:42, ...
2 years, 7 months ago (2017-08-25 08:16:58 UTC) #5
tlucas
Patch Set 2 * moved resolve_npm_dependecies-call to ensure- /update_repo * adjusted handling npm-output * don't ...
2 years, 7 months ago (2017-08-25 09:55:03 UTC) #6
Wladimir Palant
https://codereview.adblockplus.org/29526588/diff/29527639/ensure_dependencies.py File ensure_dependencies.py (right): https://codereview.adblockplus.org/29526588/diff/29527639/ensure_dependencies.py#newcode333 ensure_dependencies.py:333: resolve_npm_dependencies(target, type) For Mercurial repositories, both cloning and updating ...
2 years, 7 months ago (2017-08-25 10:01:59 UTC) #7
tlucas
Patch Set 3 Ensured single call of resolve_npm_dependencies() https://codereview.adblockplus.org/29526588/diff/29527639/ensure_dependencies.py File ensure_dependencies.py (right): https://codereview.adblockplus.org/29526588/diff/29527639/ensure_dependencies.py#newcode333 ensure_dependencies.py:333: resolve_npm_dependencies(target, ...
2 years, 7 months ago (2017-08-25 10:06:40 UTC) #8
Wladimir Palant
LGTM
2 years, 7 months ago (2017-08-25 10:13:11 UTC) #9
Vasily Kuznetsov
Hi Tristan, Looks pretty good apart from some stylistic nits (see below). Cheers, Vasily https://codereview.adblockplus.org/29526588/diff/29527641/ensure_dependencies.py ...
2 years, 7 months ago (2017-08-25 18:37:40 UTC) #10
tlucas
Patch Set 4 Removing an obsolete comment, improving readability. Good morning guys, please have another ...
2 years, 7 months ago (2017-08-28 06:53:16 UTC) #11
Wladimir Palant
Still LGTM.
2 years, 7 months ago (2017-08-28 08:00:15 UTC) #12
Vasily Kuznetsov
2 years, 7 months ago (2017-08-28 08:52:34 UTC) #13
LGTM
Sign in to reply to this message.

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