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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 9610002: remove node.js dependency by running the frog compiler in (Closed)

Created:
8 years, 9 months ago by mattsh
Modified:
8 years, 9 months ago
CC:
reviews_dartlang.org
Visibility:
Public.

Description

remove node.js dependency by running the frog compiler in DumpRenderTree instead of on node.js BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=5207

Patch Set 1 #

Patch Set 2 : fixes #

Patch Set 3 : remove blank change in frogpad.dart #

Patch Set 4 : removed print #

Patch Set 5 : updated test_options #

Total comments: 8

Patch Set 6 : frogpad reports compilation errors #

Patch Set 7 : language status #

Total comments: 4

Patch Set 8 : more fixes #

Patch Set 9 : fix frog html #

Total comments: 10

Patch Set 10 : code review small fixes and rename build_frogpad file #

Patch Set 11 : restored deleted line #

Patch Set 12 : no change #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -81 lines) Patch
M frog/dart-frog.gyp View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -5 lines 0 comments Download
M frog/scripts/bootstrap/build_frogpad_js.py View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -6 lines 0 comments Download
M frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py View 1 2 3 4 5 6 7 8 9 1 chunk +0 lines, -55 lines 0 comments Download
M tests/isolate/isolate.status View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -2 lines 0 comments Download
M tests/language/language.status View 1 2 3 4 5 6 7 8 9 10 11 7 chunks +17 lines, -9 lines 0 comments Download
M tools/testing/dart/test_options.dart View 1 2 3 4 5 6 7 1 chunk +2 lines, -2 lines 0 comments Download
M tools/testing/dart/test_suite.dart View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +13 lines, -1 line 0 comments Download
M tools/testing/frogpad/frogpad.py View 1 2 3 4 5 6 7 8 9 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 10 (0 generated)
mattsh
'frogsh' component no longer requires node.js (we use DumpRenderTree instead)
8 years, 9 months ago (2012-03-06 00:24:37 UTC) #1
Emily Fortuna
https://chromiumcodereview.appspot.com/9610002/diff/5003/frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py File frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py (left): https://chromiumcodereview.appspot.com/9610002/diff/5003/frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py#oldcode32 frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py:32: shutil.copy(os.path.join(HOME, 'scripts', 'bootstrap', 'frogsh.bat'), We don't have a DRT ...
8 years, 9 months ago (2012-03-06 00:37:52 UTC) #2
Siggi Cherem (dart-lang)
Cool! is this the last dependence on nodejs we have in the bots? Overall looks ...
8 years, 9 months ago (2012-03-06 00:41:48 UTC) #3
Emily Fortuna
lgtm, with comments we discussed a minute ago. https://chromiumcodereview.appspot.com/9610002/diff/5003/frog/dart-frog.gyp File frog/dart-frog.gyp (right): https://chromiumcodereview.appspot.com/9610002/diff/5003/frog/dart-frog.gyp#newcode90 frog/dart-frog.gyp:90: '../tools/testing/frogpad/frogpad.dart', ...
8 years, 9 months ago (2012-03-06 00:50:04 UTC) #4
kasperl
Didn't look at the details, but I like the general approach and really appreciate your ...
8 years, 9 months ago (2012-03-06 06:32:27 UTC) #5
ngeoffray
Very nice work Matt! Can't wait to run frogpad with test.py! https://chromiumcodereview.appspot.com/9610002/diff/12/frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py File frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py (right): ...
8 years, 9 months ago (2012-03-06 07:14:20 UTC) #6
mattsh
https://chromiumcodereview.appspot.com/9610002/diff/12/frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py File frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py (right): https://chromiumcodereview.appspot.com/9610002/diff/12/frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py#newcode40 frog/scripts/bootstrap/frogsh_bootstrap_wrapper.py:40: stat.S_IRGRP | stat.S_IWUSR) On 2012/03/06 07:14:20, ngeoffray wrote: > ...
8 years, 9 months ago (2012-03-06 16:17:33 UTC) #7
Bill Hesse
LGTM (dart test scripts part) https://chromiumcodereview.appspot.com/9610002/diff/11002/tools/testing/dart/test_runner.dart File tools/testing/dart/test_runner.dart (left): https://chromiumcodereview.appspot.com/9610002/diff/11002/tools/testing/dart/test_runner.dart#oldcode514 tools/testing/dart/test_runner.dart:514: stderr.add('test.dart: Compilation failed$suffix, exit ...
8 years, 9 months ago (2012-03-08 10:34:27 UTC) #8
mattsh
https://chromiumcodereview.appspot.com/9610002/diff/11002/tools/testing/dart/test_suite.dart File tools/testing/dart/test_suite.dart (right): https://chromiumcodereview.appspot.com/9610002/diff/11002/tools/testing/dart/test_suite.dart#newcode693 tools/testing/dart/test_suite.dart:693: Expect.fail("Unexpected component '$component'"); On 2012/03/08 10:34:27, Bill Hesse wrote: ...
8 years, 9 months ago (2012-03-08 16:31:41 UTC) #9
mattsh
8 years, 9 months ago (2012-03-08 16:43:28 UTC) #10 
https://chromiumcodereview.appspot.com/9610002/diff/11002/tools/testing/dart/...
File tools/testing/dart/test_runner.dart (left):

https://chromiumcodereview.appspot.com/9610002/diff/11002/tools/testing/dart/...
tools/testing/dart/test_runner.dart:514: stderr.add('test.dart: Compilation
failed$suffix, exit code $exitCode\n');
On 2012/03/08 10:34:27, Bill Hesse wrote:
> This is deleted because the absence of the "Compilation finished (step n of
> total)" is enough to say where the error is?
Good catch.  I didn't intend to delete this, put back now.

https://chromiumcodereview.appspot.com/9610002/diff/11002/tools/testing/dart/...
File tools/testing/dart/test_runner.dart (right):

https://chromiumcodereview.appspot.com/9610002/diff/11002/tools/testing/dart/...
tools/testing/dart/test_runner.dart:516: stderr.add('test.dart: Compilion
finished $suffix\n');
On 2012/03/08 10:34:27, Bill Hesse wrote:
> Compilation, not Compilion.

This spelling typo is unrelated to this CL, I'd prefer to fix in a separate CL
if that's OK.

Powered by Google App Engine
This is Rietveld 408576698