Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(456)

Issue 12088067: Second attempt to fix dart2js.bat so it looks for dart2js snapshot in right place. (Closed)

Created:
7 years, 10 months ago by aam-me
Modified:
7 years, 9 months ago
Reviewers:
ahe, kasperl, kustermann
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Second attempt to fix dart2js.bat so it looks for dart2js snapshot in right place and propagates correct exit code back to caller. BUG=dartbug.com/7663 Committed: https://code.google.com/p/dart/source/detail?r=20429

Patch Set 1 #

Patch Set 2 : Removed redundant goto. #

Patch Set 3 : Rebased #

Total comments: 2

Patch Set 4 : Removed outdated --no-use--inlining option. #

Total comments: 4

Patch Set 5 : Rebased. Picked up changes to how snapshot is used. #

Total comments: 2

Patch Set 6 : Removed redundant assignment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -18 lines) Patch
M sdk/bin/dart2js.bat View 1 2 3 4 5 1 chunk +45 lines, -8 lines 0 comments Download
M sdk/bin/dart2js_developer.bat View 1 2 1 chunk +4 lines, -10 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
aam-me
Hi, Peter, I want to retry dart2js.bat snapshot fix I tried back in https://chromiumcodereview.appspot.com/11740040/. This ...
7 years, 10 months ago (2013-01-30 04:39:49 UTC) #1
aam-me
Hi Peter, hope you have some time to look at this change. With snapshots-enabled dart2js ...
7 years, 9 months ago (2013-03-17 04:07:07 UTC) #2
ahe
Martin, Kasper, can you take a look at this? Martin, I know you don't work ...
7 years, 9 months ago (2013-03-17 22:23:22 UTC) #3
kasperl
https://chromiumcodereview.appspot.com/12088067/diff/7001/sdk/bin/dart2js.bat File sdk/bin/dart2js.bat (right): https://chromiumcodereview.appspot.com/12088067/diff/7001/sdk/bin/dart2js.bat#newcode35 sdk/bin/dart2js.bat:35: set EXTRA_VM_OPTIONS=%EXTRA_VM_OPTIONS% --no_use_inlining This seems outdated.
7 years, 9 months ago (2013-03-18 10:46:59 UTC) #4
aam-me
Thank you, Kasper! PTAL https://chromiumcodereview.appspot.com/12088067/diff/7001/sdk/bin/dart2js.bat File sdk/bin/dart2js.bat (right): https://chromiumcodereview.appspot.com/12088067/diff/7001/sdk/bin/dart2js.bat#newcode35 sdk/bin/dart2js.bat:35: set EXTRA_VM_OPTIONS=%EXTRA_VM_OPTIONS% --no_use_inlining Done. Thanks!
7 years, 9 months ago (2013-03-18 11:31:52 UTC) #5
kustermann
My bat scripting skills are not really good. But as far as I see it, ...
7 years, 9 months ago (2013-03-18 16:15:30 UTC) #6
aam-me
On 2013/03/18 16:15:30, kustermann wrote: > My bat scripting skills are not really good. But ...
7 years, 9 months ago (2013-03-18 17:19:10 UTC) #7
aam-me
Thank you, Martin. See my comments below. https://chromiumcodereview.appspot.com/12088067/diff/15001/sdk/bin/dart2js.bat File sdk/bin/dart2js.bat (right): https://chromiumcodereview.appspot.com/12088067/diff/15001/sdk/bin/dart2js.bat#newcode12 sdk/bin/dart2js.bat:12: for %%i ...
7 years, 9 months ago (2013-03-18 17:20:23 UTC) #8
kustermann
lgtm, and thanks for fixing it.
7 years, 9 months ago (2013-03-21 23:12:16 UTC) #9
aam-me
Thank you, Martin. Thanks to Peter for the heads up - Siva just recently updated ...
7 years, 9 months ago (2013-03-22 03:48:15 UTC) #10
kustermann
lgtm https://chromiumcodereview.appspot.com/12088067/diff/14002/sdk/bin/dart2js.bat File sdk/bin/dart2js.bat (right): https://chromiumcodereview.appspot.com/12088067/diff/14002/sdk/bin/dart2js.bat#newcode30 sdk/bin/dart2js.bat:30: set EXTRA_VM_OPTIONS=%EXTRA_VM_OPTIONS% I think this assignment is no ...
7 years, 9 months ago (2013-03-22 16:37:25 UTC) #11
aam-me
Thank you for the review, Martin. https://chromiumcodereview.appspot.com/12088067/diff/14002/sdk/bin/dart2js.bat File sdk/bin/dart2js.bat (right): https://chromiumcodereview.appspot.com/12088067/diff/14002/sdk/bin/dart2js.bat#newcode30 sdk/bin/dart2js.bat:30: set EXTRA_VM_OPTIONS=%EXTRA_VM_OPTIONS% You're ...
7 years, 9 months ago (2013-03-22 23:53:31 UTC) #12
aam-me
7 years, 9 months ago (2013-03-22 23:53:59 UTC) #13
Message was sent while issue was closed.
Committed patchset #6 manually as r20429 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698