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

Issue 29355271: Noissue - Don't import JavaScript modules into the global scope (includes removing unused module). (Closed)

Created:
Sept. 29, 2016, 12:05 p.m. by sergei
Modified:
Oct. 4, 2016, 10:53 a.m.
Visibility:
Public.

Description

#project abpcrawler

Patch Set 1 #

Total comments: 2

Patch Set 2 : address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -8 lines) Patch
M lib/commandLine.js View 1 chunk +1 line, -2 lines 0 comments Download
M lib/crawler.js View 1 1 chunk +4 lines, -3 lines 0 comments Download
M lib/main.js View 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 5
sergei
On 2016/09/29 10:36:10, Wladimir Palant wrote: > Why did you remove Services.jsm? It is being ...
Sept. 29, 2016, 12:44 p.m. (2016-09-29 12:44:14 UTC) #1
Wladimir Palant
On 2016/09/29 12:44:14, sergei wrote: > I have updated buildtools and have not found any ...
Sept. 29, 2016, 6:17 p.m. (2016-09-29 18:17:14 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29355271/diff/29355272/lib/crawler.js File lib/crawler.js (right): https://codereview.adblockplus.org/29355271/diff/29355272/lib/crawler.js#newcode15 lib/crawler.js:15: const {Task} = Cu.import("resource://gre/modules/Task.jsm", {}); You cannot remove Promise.jsm ...
Sept. 29, 2016, 6:19 p.m. (2016-09-29 18:19:28 UTC) #3
sergei
https://codereview.adblockplus.org/29355271/diff/29355272/lib/crawler.js File lib/crawler.js (right): https://codereview.adblockplus.org/29355271/diff/29355272/lib/crawler.js#newcode15 lib/crawler.js:15: const {Task} = Cu.import("resource://gre/modules/Task.jsm", {}); On 2016/09/29 18:19:28, Wladimir ...
Sept. 29, 2016, 9 p.m. (2016-09-29 21:00:18 UTC) #4
Wladimir Palant
Sept. 30, 2016, 6:05 a.m. (2016-09-30 06:05:12 UTC) #5
LGTM

Powered by Google App Engine
This is Rietveld