Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29363565: Issue 4552 - Drop jshydra dependency (buildtools) (Closed)

Created:
Nov. 18, 2016, 5:20 p.m. by kzar
Modified:
Nov. 30, 2016, 4:02 p.m.
CC:
kvas
Visibility:
Public.

Description

Issue 4552 - Drop jshydra dependency (buildtools)

Patch Set 1 #

Total comments: 24

Patch Set 2 : Addressed feedback, make tox pass #

Total comments: 4

Patch Set 3 : Check module exists before auto-loading it #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -13 lines) Patch
D dependencies View 1 chunk +0 lines, -2 lines 0 comments Download
M packagerChrome.py View 1 2 1 chunk +23 lines, -9 lines 0 comments Download
M releaseAutomation.py View 1 1 chunk +1 line, -0 lines 0 comments Download
A templates/modules.js.tmpl View 1 2 1 chunk +20 lines, -0 lines 0 comments Download
M tox.ini View 1 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 8
kzar
Patch Set 1 https://codereview.adblockplus.org/29363565/diff/29363566/packagerChrome.py File packagerChrome.py (right): https://codereview.adblockplus.org/29363565/diff/29363566/packagerChrome.py#newcode166 packagerChrome.py:166: modules = [] List of tuples ...
Nov. 18, 2016, 5:25 p.m. (2016-11-18 17:25:48 UTC) #1
kzar
(Publishing again since I forgot to Cc Vasily, sorry.)
Nov. 18, 2016, 5:26 p.m. (2016-11-18 17:26:55 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29363565/diff/29363566/packagerChrome.py File packagerChrome.py (right): https://codereview.adblockplus.org/29363565/diff/29363566/packagerChrome.py#newcode155 packagerChrome.py:155: args[filename][arg] = item[1] Nit: Frankly, I'd prefer `args.setdefault(filename, {})[arg] ...
Nov. 30, 2016, 11:39 a.m. (2016-11-30 11:39:33 UTC) #3
Wladimir Palant
Overlooked one more issue. https://codereview.adblockplus.org/29363565/diff/29363566/templates/modules.js.tmpl File templates/modules.js.tmpl (right): https://codereview.adblockplus.org/29363565/diff/29363566/templates/modules.js.tmpl#newcode6 templates/modules.js.tmpl:6: return exports; This should return ...
Nov. 30, 2016, 11:42 a.m. (2016-11-30 11:42:14 UTC) #4
kzar
Patch Set 2 : Addressed feedback, make tox pass https://codereview.adblockplus.org/29363565/diff/29363566/packagerChrome.py File packagerChrome.py (right): https://codereview.adblockplus.org/29363565/diff/29363566/packagerChrome.py#newcode155 packagerChrome.py:155: ...
Nov. 30, 2016, 2:29 p.m. (2016-11-30 14:29:45 UTC) #5
Wladimir Palant
https://codereview.adblockplus.org/29363565/diff/29363566/packagerChrome.py File packagerChrome.py (right): https://codereview.adblockplus.org/29363565/diff/29363566/packagerChrome.py#newcode150 packagerChrome.py:150: for item in params['metadata'].items('convert_js'): Nit: `for name, value in ...
Nov. 30, 2016, 2:54 p.m. (2016-11-30 14:54:45 UTC) #6
kzar
Patch Set 3 : Check module exists before auto-loading it https://codereview.adblockplus.org/29363565/diff/29363566/packagerChrome.py File packagerChrome.py (right): https://codereview.adblockplus.org/29363565/diff/29363566/packagerChrome.py#newcode150 ...
Nov. 30, 2016, 3:26 p.m. (2016-11-30 15:26:18 UTC) #7
Wladimir Palant
Nov. 30, 2016, 3:41 p.m. (2016-11-30 15:41:26 UTC) #8
LGTM

Powered by Google App Engine
This is Rietveld