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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 6 months ago by Wladimir Palant
Modified:
3 years, 6 months ago
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
3 years, 6 months ago (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 ...
3 years, 6 months ago (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)) ...
3 years, 6 months ago (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 ...
3 years, 6 months ago (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 ...
3 years, 6 months ago (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. ...
3 years, 6 months ago (2016-05-31 15:13:44 UTC) #6
kzar
3 years, 6 months ago (2016-05-31 15:16:24 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