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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months ago by kzar
Modified:
2 months ago
Reviewers:
tlucas, Sebastian Noack
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 ...
2 months ago (2017-10-12 11:57:28 UTC) #1
kzar
Patch Set 2 : Avoid both mapping and bundling the same file
2 months ago (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 ...
2 months ago (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 ...
2 months ago (2017-10-13 07:25:21 UTC) #4
tlucas
LGTM on the python part (and most js parts) - i assume you verified the ...
2 months ago (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? ...
2 months ago (2017-10-13 13:44:17 UTC) #6
Sebastian Noack
2 months ago (2017-10-14 18:39:41 UTC) #7
LGTM
Sign in to reply to this message.

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