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

Issue 29345407: Issue 4090 - Make require() load modules lazily (Closed)

Created:
May 31, 2016, 11:37 a.m. by Wladimir Palant
Modified:
June 1, 2016, 12:07 p.m.
Reviewers:
kzar
CC:
Sebastian Noack
Visibility:
Public.

Description

This is missing dependency update, will add it once #4088 lands. Repository: hg.adblockplus.org/adblockpluschrome

Patch Set 1 #

Total comments: 5

Patch Set 2 : Addressed comments and fixed list of initially loaded modules #

Total comments: 3

Patch Set 3 : Added dependency update #

Unified diffs Side-by-side diffs Delta from patch set Stats (+126 lines, -118 lines) Patch
M dependencies View 1 2 1 chunk +1 line, -1 line 0 comments Download
M ensure_dependencies.py View 1 2 4 chunks +88 lines, -89 lines 0 comments Download
M lib/compat.js View 1 1 chunk +8 lines, -1 line 0 comments Download
M metadata.common View 1 1 chunk +29 lines, -27 lines 0 comments Download

Messages

Total messages: 7
Wladimir Palant
May 31, 2016, 11:37 a.m. (2016-05-31 11:37:41 UTC) #1
Sebastian Noack
https://codereview.adblockplus.org/29345407/diff/29345408/background.js File background.js (right): https://codereview.adblockplus.org/29345407/diff/29345408/background.js#newcode18 background.js:18: // Make sure that the required modules get loaded ...
May 31, 2016, 11:42 a.m. (2016-05-31 11:42:13 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29345407/diff/29345408/lib/compat.js File lib/compat.js (right): https://codereview.adblockplus.org/29345407/diff/29345408/lib/compat.js#newcode25 lib/compat.js:25: if (typeof result == "function" && !(module in require.loaded)) ...
May 31, 2016, 11:44 a.m. (2016-05-31 11:44:27 UTC) #3
Wladimir Palant
https://codereview.adblockplus.org/29345407/diff/29345408/background.js File background.js (right): https://codereview.adblockplus.org/29345407/diff/29345408/background.js#newcode18 background.js:18: // Make sure that the required modules get loaded ...
May 31, 2016, 1:47 p.m. (2016-05-31 13:47:55 UTC) #4
kzar
LGTM https://codereview.adblockplus.org/29345407/diff/29345408/background.js File background.js (right): https://codereview.adblockplus.org/29345407/diff/29345408/background.js#newcode18 background.js:18: // Make sure that the required modules get ...
May 31, 2016, 2:54 p.m. (2016-05-31 14:54:09 UTC) #5
Wladimir Palant
The buildtools changes landed so I've uploaded the final patch now, with the dependency update. ...
May 31, 2016, 3:13 p.m. (2016-05-31 15:13:44 UTC) #6
kzar
May 31, 2016, 3:16 p.m. (2016-05-31 15:16:24 UTC) #7
LGTM

Powered by Google App Engine
This is Rietveld