Issue 573863003: simple index.html files for folders that contain entry points

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+229 lines, -2 lines)			Patch
M	pkg/pkg.status	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	pkg/polymer/CHANGELOG.md	View	1 2 3	1 chunk	+5 lines, -0 lines	0 comments	Download
A	pkg/polymer/lib/src/build/index_page_builder.dart	View	1 2 3 4	1 chunk	+104 lines, -0 lines	0 comments	Download
M	pkg/polymer/lib/transformer.dart	View		3 chunks	+6 lines, -1 line	0 comments	Download
M	pkg/polymer/pubspec.yaml	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
A	pkg/polymer/test/build/index_page_builder_test.dart	View	1 2	1 chunk	+112 lines, -0 lines	0 comments	Download

Messages

Total messages: 7 (1 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Siggi Cherem (dart-lang)

very nice. Some small suggestions below https://codereview.chromium.org/573863003/diff/20001/pkg/polymer/lib/src/build/index_page_builder.dart File pkg/polymer/lib/src/build/index_page_builder.dart (right): https://codereview.chromium.org/573863003/diff/20001/pkg/polymer/lib/src/build/index_page_builder.dart#newcode27 pkg/polymer/lib/src/build/index_page_builder.dart:27: return ''; nit: ...

6 years, 3 months ago (2014-09-16 19:40:50 UTC) #3

jakemac

https://codereview.chromium.org/573863003/diff/20001/pkg/polymer/lib/src/build/index_page_builder.dart File pkg/polymer/lib/src/build/index_page_builder.dart (right): https://codereview.chromium.org/573863003/diff/20001/pkg/polymer/lib/src/build/index_page_builder.dart#newcode27 pkg/polymer/lib/src/build/index_page_builder.dart:27: return ''; Changed to 'all_entry_points' On 2014/09/16 19:40:50, Siggi ...

6 years, 3 months ago (2014-09-16 21:17:05 UTC) #4

https://codereview.chromium.org/573863003/diff/20001/pkg/polymer/lib/src/buil...
File pkg/polymer/lib/src/build/index_page_builder.dart (right):

https://codereview.chromium.org/573863003/diff/20001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/index_page_builder.dart:27: return '';
Changed to 'all_entry_points'

On 2014/09/16 19:40:50, Siggi Cherem (dart-lang) wrote:
> nit: it might feel less magical to use a non-empty string?

https://codereview.chromium.org/573863003/diff/20001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/index_page_builder.dart:90: var doc = new
Document.html('<!DOCTYPE html><html><body>'
On 2014/09/16 19:40:50, Siggi Cherem (dart-lang) wrote:
> change to just use StringBuffer? It seems there is no need to actually parse
the
> generated code as an HTML Document since we are anyways serializing it just
> below.
> 

Done.

https://codereview.chromium.org/573863003/diff/20001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/index_page_builder.dart:91: '<h1>Entry
points</h1><ul></ul></body></html>');
Flattened, I think that is the easiest approach and seems reasonable since the
total # of entry points is usually not large.

On 2014/09/16 19:40:50, Siggi Cherem (dart-lang) wrote:
> should we have a different title or divide the list in two? so that
directories
> are not labeled 'Entry points'?
> 
> The other option would be to flatten the list and include all entrypoints
under
> web/ recursively in web/index.html

https://codereview.chromium.org/573863003/diff/20001/pkg/polymer/test/build/i...
File pkg/polymer/test/build/index_page_builder_test.dart (right):

https://codereview.chromium.org/573863003/diff/20001/pkg/polymer/test/build/i...
pkg/polymer/test/build/index_page_builder_test.dart:43: testPhases('doesn\'t
overwrite existing pages', phases, {
Including index.html since I am flattening everything now anyways, and added a
few other tests.

On 2014/09/16 19:40:50, Siggi Cherem (dart-lang) wrote:
> let's add some additional tests:
>  - A that mixes both generated and non-generated `index.html`s (this has none
> generated and the above one has all generated). In that case, do we want to
> include index.html in the list of a parent folder? or just include the folder
> link (since it will open index.html anyways)?
> 
>  - a test were there are entrypoints under web/ and example/

Siggi Cherem (dart-lang)

cool, lgtm - just a suggestion to simplify the sorting https://codereview.chromium.org/573863003/diff/40001/pkg/polymer/lib/src/build/index_page_builder.dart File pkg/polymer/lib/src/build/index_page_builder.dart (right): https://codereview.chromium.org/573863003/diff/40001/pkg/polymer/lib/src/build/index_page_builder.dart#newcode13 ...

6 years, 3 months ago (2014-09-17 20:32:54 UTC) #5

jakemac

https://codereview.chromium.org/573863003/diff/40001/pkg/polymer/lib/src/build/index_page_builder.dart File pkg/polymer/lib/src/build/index_page_builder.dart (right): https://codereview.chromium.org/573863003/diff/40001/pkg/polymer/lib/src/build/index_page_builder.dart#newcode13 pkg/polymer/lib/src/build/index_page_builder.dart:13: import 'package:html5lib/dom.dart'; On 2014/09/17 20:32:54, Siggi Cherem (dart-lang) wrote: ...

6 years, 3 months ago (2014-09-17 22:14:01 UTC) #6

jakemac

6 years, 3 months ago (2014-09-17 22:14:22 UTC) #7

Message was sent while issue was closed.

Committed patchset #5 (id:80001) manually as 40408 (presubmit successful).

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Issue 573863003: simple index.html files for folders that contain entry points (Closed)

Description

Patch Set 1 #

Patch Set 2 : show subdirectories as well #

Patch Set 3 : review updates #

Patch Set 4 : update pubspec/changelog #

Patch Set 5 : remove unused import #

Messages