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

Issue 103483002: add back dart:mirrors content, remove dart:isolate, misc. tweaks/updates (Closed)

Created:
7 years ago by Kathy Walrath
Modified:
7 years ago
Reviewers:
Emily Fortuna, sethladd
Base URL:
https://github.com/dart-lang/dart-up-and-running-book.git@master
Visibility:
Public.

Description

add back dart:mirrors content, remove dart:isolate, misc. tweaks/updates staged at: https://book-dot-dart-lang.appspot.com/docs/dart-up-and-running/ R=sethladd@google.com Committed: 1c17ea4

Patch Set 1 #

Patch Set 2 : move a misplaced comment #

Total comments: 2

Patch Set 3 : emphasize MirrorsUsed lots #

Total comments: 16
Unified diffs Side-by-side diffs Delta from patch set Stats (+339 lines, -216 lines) Patch
M bookinfo.xml View 2 chunks +2 lines, -2 lines 0 comments Download
M ch00.xml View 4 chunks +139 lines, -64 lines 0 comments Download
M ch01.xml View 4 chunks +9 lines, -10 lines 0 comments Download
M ch02.xml View 2 chunks +1 line, -4 lines 0 comments Download
M ch03.xml View 1 2 4 chunks +174 lines, -135 lines 16 comments Download
M code/ch03/mirrors.dart View 3 chunks +14 lines, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
Kathy Walrath
7 years ago (2013-12-04 01:49:59 UTC) #1
sethladd
lgtm once we add the warning for @MirrorsUsed https://chromiumcodereview.appspot.com/103483002/diff/20001/ch00.xml File ch00.xml (right): https://chromiumcodereview.appspot.com/103483002/diff/20001/ch00.xml#newcode213 ch00.xml:213: Use ...
7 years ago (2013-12-04 19:06:01 UTC) #2
Kathy Walrath
Committed patchset #3 manually as r1c17ea4 (presubmit successful).
7 years ago (2013-12-04 19:29:47 UTC) #3
Emily Fortuna
https://chromiumcodereview.appspot.com/103483002/diff/40001/ch03.xml File ch03.xml (right): https://chromiumcodereview.appspot.com/103483002/diff/40001/ch03.xml#newcode1242 ch03.xml:1242: <para>The Math library provides optimized <literal>max()</literal> and what do ...
7 years ago (2013-12-05 21:55:11 UTC) #4
Kathy Walrath
7 years ago (2013-12-13 00:48:20 UTC) #5
Message was sent while issue was closed.
Finally incorporated Emily's comments (except for a few, notably cascade
indentation, which really should be 4 spaces).

Thanks, Emily!

https://chromiumcodereview.appspot.com/103483002/diff/40001/ch03.xml
File ch03.xml (right):

https://chromiumcodereview.appspot.com/103483002/diff/40001/ch03.xml#newcode1242
ch03.xml:1242: <para>The Math library provides optimized
<literal>max()</literal> and
On 2013/12/05 21:55:11, Emily Fortuna wrote:
> what do you mean by "optimized" methods here?

Done.

https://chromiumcodereview.appspot.com/103483002/diff/40001/ch03.xml#newcode1318
ch03.xml:1318: <para>For scalable, higher level approaches to web app UIs, see
<ulink
On 2013/12/05 21:55:11, Emily Fortuna wrote:
> might consider removing "scalable" ... that seems to suggest that the
dart:html
> library is not really a viable option to use on its own (yes, obviously
Polymer
> helps make it better, but we don't want to suggest that dart:html is not good
to
> work with directly)

Done.

https://chromiumcodereview.appspot.com/103483002/diff/40001/ch03.xml#newcode1345
ch03.xml:1345: <literal moreinfo="none">document</literal> property (a Document
On 2013/12/05 21:55:11, Emily Fortuna wrote:
> Consider just saying "Document object" instead of "property (a Document
> object)". Users can determine it's a property by looking at the documentation,
> and it's assumed if the Window has it, you can access it in some way.

Done.

https://chromiumcodereview.appspot.com/103483002/diff/40001/ch03.xml#newcode1348
ch03.xml:1348: data), <literal>requestAnimationFrame()</literal> (for
animations), and
On 2013/12/05 21:55:11, Emily Fortuna wrote:
> actually the current Dart idiom is to use "animationFrame"
>
https://api.dartlang.org/docs/channels/be/latest/dart_html/Window.html#animat...
> which returns a Future, when possible.

I switched to not use the () and code font, since I wanted to refer to the API
in general, not to our implementation of the API.

https://chromiumcodereview.appspot.com/103483002/diff/40001/ch03.xml#newcode1512
ch03.xml:1512: List&lt;Node&gt;. Once you have this list, you can use the usual
List
On 2013/12/05 21:55:11, Emily Fortuna wrote:
> maybe add "as opposed to 'children' which just returns the child elements."

Done.

https://chromiumcodereview.appspot.com/103483002/diff/40001/ch03.xml#newcode1574
ch03.xml:1574: ..id = 'message2'
On 2013/12/05 21:55:11, Emily Fortuna wrote:
> stylistic note: at google, when the line is a method cascade, we only indent
+2
> spaces from the previous line rather than the 4

I had to look this one up. According to
https://code.google.com/p/dart/issues/detail?id=8175, the style guide has been
amended to specify 4 spaces for cascades:

https://www.dartlang.org/articles/style-guide/#do-use-four-spaces-for-method-...

Powered by Google App Engine
This is Rietveld 408576698