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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1645343002: Builds / serves multiple HTML files. (Closed)

Created:
4 years, 10 months ago by vsm
Modified:
4 years, 10 months ago
Reviewers:
Jennifer Messerly
CC:
dev-compiler+reviews_dartlang.org, yjbanov
Base URL:
https://github.com/dart-lang/dev_compiler.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Builds / serves multiple HTML files. With this, I can build multiple Angular samples simultaneously: [dist/dart/playground/build/web] dart ~/git/dev_compiler/bin/dartdevc.dart -o /tmp/ddc-out --force-compile src/*/index.html Fixes #430 R=jmesserly@google.com Committed: https://github.com/dart-lang/dev_compiler/commit/0ad43572ecdec86389a716360aec6cf658beab55

Patch Set 1 #

Patch Set 2 : Cleanup #

Total comments: 6

Patch Set 3 : Address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+83 lines, -1091 lines) Patch
M lib/src/codegen/html_codegen.dart View 1 2 2 chunks +9 lines, -3 lines 0 comments Download
M lib/src/compiler.dart View 1 2 1 chunk +4 lines, -4 lines 0 comments Download
M lib/src/options.dart View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M lib/src/server/dependency_graph.dart View 1 2 2 chunks +17 lines, -1 line 0 comments Download
M lib/src/server/server.dart View 1 2 4 chunks +22 lines, -13 lines 0 comments Download
M lib/src/utils.dart View 1 2 1 chunk +6 lines, -1 line 0 comments Download
M test/codegen/expect/collection/equality.txt View 1 2 1 chunk +0 lines, -9 lines 0 comments Download
M test/codegen/expect/collection/src/canonicalized_map.js View 1 2 3 chunks +8 lines, -5 lines 0 comments Download
M test/codegen/expect/collection/src/canonicalized_map.txt View 1 2 1 chunk +2 lines, -4 lines 0 comments Download
M test/codegen/expect/collection/src/queue_list.txt View 1 2 1 chunk +8 lines, -6 lines 0 comments Download
D test/codegen/expect/collection/src/unmodifiable_wrappers.js View 1 2 1 chunk +0 lines, -229 lines 0 comments Download
M test/codegen/expect/collection/src/unmodifiable_wrappers.txt View 1 2 1 chunk +6 lines, -4 lines 0 comments Download
M test/codegen/expect/collection/src/wrappers.txt View 1 2 1 chunk +1 line, -1 line 0 comments Download
D test/codegen/expect/collection/wrappers.js View 1 2 1 chunk +0 lines, -809 lines 0 comments Download
M test/codegen/expect/language-all.js View 1 2 Binary file 0 comments Download

Messages

Total messages: 7 (3 generated)
vsm
4 years, 10 months ago (2016-01-29 15:50:23 UTC) #3
Jennifer Messerly
lgtm https://codereview.chromium.org/1645343002/diff/20001/lib/src/compiler.dart File lib/src/compiler.dart (right): https://codereview.chromium.org/1645343002/diff/20001/lib/src/compiler.dart#newcode310 lib/src/compiler.dart:310: file.openSync(mode: FileMode.WRITE) silly style nit: curious about the ...
4 years, 10 months ago (2016-01-29 16:49:49 UTC) #4
vsm
Committed patchset #3 (id:40001) manually as 0ad43572ecdec86389a716360aec6cf658beab55 (presubmit successful).
4 years, 10 months ago (2016-01-29 18:13:37 UTC) #6
vsm
4 years, 10 months ago (2016-01-29 18:13:51 UTC) #7
Message was sent while issue was closed. 
https://codereview.chromium.org/1645343002/diff/20001/lib/src/compiler.dart
File lib/src/compiler.dart (right):

https://codereview.chromium.org/1645343002/diff/20001/lib/src/compiler.dart#n...
lib/src/compiler.dart:310: file.openSync(mode: FileMode.WRITE)
On 2016/01/29 16:49:49, John Messerly wrote:
> silly style nit: curious about the mix between cascades and non-cascades. IMO,
> it'd be better to go all in on either one. Dartformat will do a better job if
> it's one big cascade:
> 
> new File(outputFile)
>   ..createSync(recursive: true)
>   ..openSync(mode: FileMode.WRITE)
>   ...
> 
> 
> This would work too:
> 
> var file = new File(outputFile);
> file.createSync(recursive: true);
> file.openSync(mode: FileMode.WRITE);
> file. ...

Done.

https://codereview.chromium.org/1645343002/diff/20001/lib/src/server/server.dart
File lib/src/server/server.dart (right):

https://codereview.chromium.org/1645343002/diff/20001/lib/src/server/server.d...
lib/src/server/server.dart:126: file.writeAsStringSync(output);
On 2016/01/29 16:49:49, John Messerly wrote:
> same comment, it'd be nice to go all in on the cascades, or not :)

Done.

https://codereview.chromium.org/1645343002/diff/20001/lib/src/utils.dart
File lib/src/utils.dart (right):

https://codereview.chromium.org/1645343002/diff/20001/lib/src/utils.dart#newc...
lib/src/utils.dart:319: var entryDir = entryPath.endsWith('.dart') ||
entryPath.endsWith('.html')
On 2016/01/29 16:49:49, John Messerly wrote:
> A comment might be nice here, not sure I follow what the change is.

Done.

Powered by Google App Engine
This is Rietveld 408576698