Issue 10126002: Enable html_dartium to be built as part of dartium build

Issue 10126002: Enable html_dartium to be built as part of dartium build (Closed)

Created:
8 years, 8 months ago by vsm

Modified:
8 years, 8 months ago

Reviewers:
sra, Anton Muhin, antonm, sra1

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Enable html_dartium to be built as part of dartium build Committed: https://code.google.com/p/dart/source/detail?r=6752

Patch Set 2 : Address comments #

Patch Set 3 : Addressed comments #

Total comments: 3

Created: 8 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+25 lines, -16 lines)			Patch
M	lib/dom/scripts/dartdomgenerator.py	View	1	2 chunks	+3 lines, -0 lines	1 comment	Download
M	lib/dom/scripts/dartgenerator.py	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	lib/dom/scripts/systemhtml.py	View		3 chunks	+6 lines, -1 line	1 comment	Download
M	lib/dom/templates/html/dartium/html_dartium.darttemplate	View		1 chunk	+14 lines, -14 lines	1 comment	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Anton Muhin

LGTM https://chromiumcodereview.appspot.com/10126002/diff/1/lib/dom/scripts/dartdomgenerator.py File lib/dom/scripts/dartdomgenerator.py (right): https://chromiumcodereview.appspot.com/10126002/diff/1/lib/dom/scripts/dartdomgenerator.py#newcode121 lib/dom/scripts/dartdomgenerator.py:121: if create_single_file: let's use an early return here, ...

8 years, 8 months ago (2012-04-19 10:23:14 UTC) #2

sra1

8 years, 8 months ago (2012-04-19 18:17:10 UTC) #3

Some minor suggestions.

lgtm

https://chromiumcodereview.appspot.com/10126002/diff/6001/lib/dom/scripts/dar...
File lib/dom/scripts/dartdomgenerator.py (right):

https://chromiumcodereview.appspot.com/10126002/diff/6001/lib/dom/scripts/dar...
lib/dom/scripts/dartdomgenerator.py:121: def GenerateSingleFile(systems):
Since many systems are active in one run, 'Single' is a bit of a misnomer.

MovePackagedFilesToDestinations ?

https://chromiumcodereview.appspot.com/10126002/diff/6001/lib/dom/scripts/sys...
File lib/dom/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10126002/diff/6001/lib/dom/scripts/sys...
lib/dom/scripts/systemhtml.py:1193: AUXILIARY_DIR=MassagePath(auxiliary_dir),
See template for suggestion.

https://chromiumcodereview.appspot.com/10126002/diff/6001/lib/dom/templates/h...
File lib/dom/templates/html/dartium/html_dartium.darttemplate (right):

https://chromiumcodereview.appspot.com/10126002/diff/6001/lib/dom/templates/h...
lib/dom/templates/html/dartium/html_dartium.darttemplate:20:
#source('$AUXILIARY_DIR/../../html/src/shared_FactoryProviders.dart');
I think this would be cleaner with some symmetry between dom and html sources.

Either two defines:

$DOM_SRC_DIR/KeyName.dart
$HTML_SRC_DIR/Device.dart

Or a common root
$LIB_DIR/dom/src/KeyName.dart
$LIB_DIR/html/src/Device.dart

Expand Messages | Collapse Messages