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

Issue 8616119: Use require() properly instead of importing all symbols of a module (Closed)

Created:
Oct. 22, 2012, 8:48 a.m. by Wladimir Palant
Modified:
Oct. 22, 2012, 1:03 p.m.
Reviewers:
Felix Dahlke
Visibility:
Public.

Description

This should make your UI porting simpler

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -18 lines) Patch
M background.js View 1 chunk +17 lines, -11 lines 0 comments Download
M options.js View 1 chunk +18 lines, -6 lines 0 comments Download
M popup.js View 1 chunk +4 lines, -1 line 0 comments Download

Messages

Total messages: 4
Wladimir Palant
Oct. 22, 2012, 8:48 a.m. (2012-10-22 08:48:47 UTC) #1
Felix Dahlke
LGTM. We may want to make sure that only modules actually used are imported in ...
Oct. 22, 2012, 11:56 a.m. (2012-10-22 11:56:25 UTC) #2
Wladimir Palant
On 2012/10/22 11:56:25, Felix H. Dahlke wrote: > We may want to make sure that ...
Oct. 22, 2012, 11:58 a.m. (2012-10-22 11:58:47 UTC) #3
Felix Dahlke
Oct. 22, 2012, 11:59 a.m. (2012-10-22 11:59:59 UTC) #4
On 2012/10/22 11:58:47, Wladimir Palant wrote:
> On 2012/10/22 11:56:25, Felix H. Dahlke wrote:
> > We may want to make sure that only modules actually used are imported in
> > background.js/options.js for clarity.
> 
> I checked that - from what I can tell, everything imported now is also used
> (with BlockingFilter as one ugly case, it is being imported in background.js
but
> used in webRequest.js).

OK, LGTM.

Powered by Google App Engine
This is Rietveld