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

Issue 6256752412590080: Issue 301 - jshydra: Handle for .. of .. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 9 months ago by tschuster
Modified:
5 years, 9 months ago
Reviewers:
Wladimir Palant
Visibility:
Public.

Description

jshydra: Handle for .. of ..

Patch Set 1 #

Total comments: 1

Patch Set 2 : autotest now passes for me #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -33 lines) Patch
M autotest/abprewrite_source.js View 1 1 chunk +6 lines, -4 lines 0 comments Download
M jshydra.js View 1 chunk +1 line, -1 line 0 comments Download
M scripts/abprewrite.js View 1 2 chunks +34 lines, -28 lines 0 comments Download
M scripts/astDecompile.js View 1 chunk +11 lines, -0 lines 0 comments Download

Messages

Total messages: 8
tschuster
review please
5 years, 9 months ago (2014-04-10 17:10:49 UTC) #1
Wladimir Palant
Looks good except for the change I commented on. http://codereview.adblockplus.org/6256752412590080/diff/5629499534213120/jshydra.js File jshydra.js (right): http://codereview.adblockplus.org/6256752412590080/diff/5629499534213120/jshydra.js#newcode43 jshydra.js:43: ...
5 years, 9 months ago (2014-04-10 17:15:10 UTC) #2
tschuster
On 2014/04/10 17:15:10, Wladimir Palant wrote: > Looks good except for the change I commented ...
5 years, 9 months ago (2014-04-10 18:06:48 UTC) #3
Wladimir Palant
LGTM Note that we are currently using SpiderMonkey 17 and not updating - starting with ...
5 years, 9 months ago (2014-04-11 05:42:59 UTC) #4
Wladimir Palant
Actually, no - that change is incomplete. autotest.py will fail this way, please adjust autotest/abprewrite_source.js, ...
5 years, 9 months ago (2014-04-11 05:58:48 UTC) #5
tschuster
On 2014/04/11 05:42:59, Wladimir Palant wrote: > LGTM > > Note that we are currently ...
5 years, 9 months ago (2014-04-11 22:26:15 UTC) #6
tschuster
autotest now passes for me
5 years, 9 months ago (2014-04-11 22:51:23 UTC) #7
Wladimir Palant
5 years, 9 months ago (2014-04-12 07:38:17 UTC) #8
LGTM

Actually, it seems that the parser change I was worried about was reverted - let
statements are no longer being produced. I tried the current nightly from
https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-11-03...
and it produced exactly the same file as the old SpiderMonkey 17 build we were
using before. I reopened https://issues.adblockplus.org/ticket/298 - feel free
to take it.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5