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

Issue 29574582: Issue 5535 - Replace our module system with webpack (Closed)

Created:
Oct. 12, 2017, 11:53 a.m. by kzar
Modified:
Oct. 15, 2017, 1:11 p.m.
Reviewers:
Sebastian Noack, tlucas
CC:
Wladimir Palant
Visibility:
Public.

Description

Issue 5535 - Replace our module system with webpack This change already landed, but then was reverted. The previous review is here https://codereview.adblockplus.org/29549786/

Patch Set 1 #

Total comments: 1

Patch Set 2 : Avoid both mapping and bundling the same file #

Total comments: 2

Patch Set 3 : Addressed nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+263 lines, -99 lines) Patch
A info.js View 1 chunk +1 line, -0 lines 0 comments Download
A info-loader.js View 1 chunk +28 lines, -0 lines 0 comments Download
M package.json View 1 chunk +3 lines, -1 line 0 comments Download
M packagerChrome.py View 1 2 3 chunks +61 lines, -47 lines 0 comments Download
M packagerEdge.py View 1 chunk +2 lines, -2 lines 0 comments Download
D templates/modules.js.tmpl View 1 chunk +0 lines, -49 lines 0 comments Download
A webpack_runner.js View 1 1 chunk +168 lines, -0 lines 0 comments Download

Messages

Total messages: 7
kzar
Patch Set 1 https://codereview.adblockplus.org/29574582/diff/29574583/webpack_runner.js File webpack_runner.js (right): https://codereview.adblockplus.org/29574582/diff/29574583/webpack_runner.js#newcode140 webpack_runner.js:140: let relativeFilepath = path.relative("/", filepath); These ...
Oct. 12, 2017, 11:57 a.m. (2017-10-12 11:57:28 UTC) #1
kzar
Patch Set 2 : Avoid both mapping and bundling the same file
Oct. 12, 2017, 6:02 p.m. (2017-10-12 18:02:42 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29574582/diff/29574651/packagerChrome.py File packagerChrome.py (right): https://codereview.adblockplus.org/29574582/diff/29574651/packagerChrome.py#newcode203 packagerChrome.py:203: del files[to_ignore] You can use files.pop(to_ignore, None), then you ...
Oct. 12, 2017, 11:23 p.m. (2017-10-12 23:23:28 UTC) #3
kzar
Patch Set 3 : Addressed nit https://codereview.adblockplus.org/29574582/diff/29574651/packagerChrome.py File packagerChrome.py (right): https://codereview.adblockplus.org/29574582/diff/29574651/packagerChrome.py#newcode203 packagerChrome.py:203: del files[to_ignore] On ...
Oct. 13, 2017, 7:25 a.m. (2017-10-13 07:25:21 UTC) #4
tlucas
LGTM on the python part (and most js parts) - i assume you verified the ...
Oct. 13, 2017, 1:40 p.m. (2017-10-13 13:40:02 UTC) #5
kzar
On 2017/10/13 13:40:02, tlucas wrote: > i assume you verified the absence of redundant copies? ...
Oct. 13, 2017, 1:44 p.m. (2017-10-13 13:44:17 UTC) #6
Sebastian Noack
Oct. 14, 2017, 6:39 p.m. (2017-10-14 18:39:41 UTC) #7
LGTM

Powered by Google App Engine
This is Rietveld