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

Issue 29338486: Issue 3822 - Fix strict-mode in generated modules (Closed)

Created:
March 17, 2016, 1:42 p.m. by Sebastian Noack
Modified:
March 17, 2016, 4:12 p.m.
Reviewers:
kzar
Visibility:
Public.

Description

Issue 3822 - Fix strict-mode in generated modules

Patch Set 1 : #

Total comments: 4

Patch Set 2 : Wrapped long line #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M scripts/abprewrite.js View 1 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 5
Sebastian Noack
March 17, 2016, 1:48 p.m. (2016-03-17 13:48:39 UTC) #1
kzar
https://codereview.adblockplus.org/29338486/diff/29338489/scripts/abprewrite.js File scripts/abprewrite.js (right): https://codereview.adblockplus.org/29338486/diff/29338489/scripts/abprewrite.js#newcode561 scripts/abprewrite.js:561: decompileAST(ast).replace(/^("use strict";\n)?/, "$1var exports = {};\n") + I guess ...
March 17, 2016, 1:56 p.m. (2016-03-17 13:56:47 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29338486/diff/29338489/scripts/abprewrite.js File scripts/abprewrite.js (right): https://codereview.adblockplus.org/29338486/diff/29338489/scripts/abprewrite.js#newcode561 scripts/abprewrite.js:561: decompileAST(ast).replace(/^("use strict";\n)?/, "$1var exports = {};\n") + On 2016/03/17 ...
March 17, 2016, 2:10 p.m. (2016-03-17 14:10:26 UTC) #3
kzar
LGTM (Did you test that this doesn't break things as strict mode wasn't being enforced ...
March 17, 2016, 2:14 p.m. (2016-03-17 14:14:28 UTC) #4
Sebastian Noack
March 17, 2016, 4:12 p.m. (2016-03-17 16:12:11 UTC) #5
On 2016/03/17 14:14:28, kzar wrote:
> LGTM
> 
> (Did you test that this doesn't break things as strict mode wasn't being
> enforced before?)

I did test it as good as I could with reasonable effort. But yeah, given where
we already use strict-mode meanwhile, covering every single line of code
effected is virtually impossible. Let's hope that if this uncovers any issues,
we will find them before the next release. ;)

Powered by Google App Engine
This is Rietveld