Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
111463004:
Adding library level docs for async, io, convert and fixing a bug in dart:core (Closed)
|
Created:
7 years ago by mem Modified:
6 years, 11 months ago CC:
reviews_dartlang.org Visibility:
Public. |
DescriptionAdding library level docs for async, io, convert and fixing a bug in dart:core
BUG=
R=lrn@google.com, sgjesse@google.com
Committed: https://code.google.com/p/dart/source/detail?r=31421
Patch Set 1 #
Total comments: 41
Patch Set 2 : changes based on kathyw's feedback #
Total comments: 44
Patch Set 3 : changes based on comments from Soren and Lasse #Patch Set 4 : fixed websocket text and sample #
Total comments: 14
Patch Set 5 : changes based on comments from sgjesse #Patch Set 6 : insurance #
Messages
Total messages: 14 (0 generated)
Some library level documentation for dart:async, dart:convert, dart:io. Plus a bug fix for dart:core. PST is in the winter and PDT is in the summer. Who knew. Thanks. mem
nitty writing stuff. Someone else will do a technical review, right? https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.dart File sdk/lib/async/async.dart (right): https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:9: * Most programs need to use [Future]s and [Stream]s, which I'd delete "which are often used together". https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:12: * Understanding Futures and Streams is a pre-requeiste for requeiste: sp I'd combine the previous paragraph and this one. Or maybe just delete the previous one. Delete all the unnecessary wordses! https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:36: * that is printed. I'd split this up. ("that is printed" confused me) Maybe: In this example, the fibonacci function returns a number, and the callback function prints that number. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:44: * and a stream of bytes read from a file. Put commas around "such as mouse clicks" so it's clear the bytes aren't user-generated events. Or switch the two examples around. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:56: * Here, the code uses an UTF8 decoder (provided in the [dart:convert] library) an UTF8 -> a UTF8 https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:57: * because the file is a simple text file. the "because" didn't make sense to me. Maybe: to convert the file's UTF8-encoded text data into a Dart string. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:60: * in a web app. This code listens for mouse clicks on a button. This -> The following AND/OR use a : instead of a . https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:66: * * For a brief overview to asynchronous programming in Dart, see the to -> of Rewrite to match the other resources (Title: Description). https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:70: * * [Using Future-Based APIs] Using -> Use https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/convert/conve... File sdk/lib/convert/convert.dart (right): https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/convert/conve... sdk/lib/convert/convert.dart:21: * Converters are often used in conjunction with streams in conjunction with -> with https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/convert/conve... sdk/lib/convert/convert.dart:22: * transforming the data that comes through the stream transforming -> to transform https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/convert/conve... sdk/lib/convert/convert.dart:42: * See the documentation for the [Codec] and the [Converter] classes and the -> and https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart File sdk/lib/io/io.dart (right): https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:21: * Consider it a pre-requisite to learn about Futures and Streams pre-requisite -> prerequisite "Consider it ..." seems weak. Actually, I'd just delete that sentence and most of the next. (just leave in the mention of dart:async, and put the links to [Future] and [Stream] in the previous sentence. And delete "import 'dart:async'". https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:33: * Because these methods Maybe: Because file system access can be slow, these methods are asynchronous and return a Future. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:48: * A [Directory] object represents a directory in the native file system. This is choppy. Maybe: Each instance of [File] or [Directory] represents a file or directory, respectively, in the native file system. Both File and Directory are subclasses of .... https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:50: * You can manipulate the file sytem through objects of these types. sytem! https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:86: * To read text synchronously from the command-line command-line -> command line (since it's not an adjectival phrase modifying a noun) https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:91: * ## Other Resources Resources -> resources https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:99: * (https://www.dartlang.org/docs/tutorials/dartio/). I think the URL should be either dart-io or io. probably just io...
did everything. Now to get engineering feedback. mem https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.dart File sdk/lib/async/async.dart (right): https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:9: * Most programs need to use [Future]s and [Stream]s, which On 2013/12/16 21:18:48, Kathy Walrath wrote: > I'd delete "which are often used together". Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:12: * Understanding Futures and Streams is a pre-requeiste for On 2013/12/16 21:18:48, Kathy Walrath wrote: > requeiste: sp > > I'd combine the previous paragraph and this one. Or maybe just delete the > previous one. Delete all the unnecessary wordses! Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:36: * that is printed. On 2013/12/16 21:18:48, Kathy Walrath wrote: > I'd split this up. ("that is printed" confused me) Maybe: > > In this example, the fibonacci function returns a number, > and the callback function prints that number. Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:44: * and a stream of bytes read from a file. On 2013/12/16 21:18:48, Kathy Walrath wrote: > Put commas around "such as mouse clicks" so it's clear the bytes aren't > user-generated events. > > Or switch the two examples around. Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:56: * Here, the code uses an UTF8 decoder (provided in the [dart:convert] library) On 2013/12/16 21:18:48, Kathy Walrath wrote: > an UTF8 -> a UTF8 Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:57: * because the file is a simple text file. On 2013/12/16 21:18:48, Kathy Walrath wrote: > the "because" didn't make sense to me. > > Maybe: > > to convert the file's UTF8-encoded text data into a Dart string. Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:60: * in a web app. This code listens for mouse clicks on a button. On 2013/12/16 21:18:48, Kathy Walrath wrote: > This -> The following > > AND/OR use a : instead of a . Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:66: * * For a brief overview to asynchronous programming in Dart, see the On 2013/12/16 21:18:48, Kathy Walrath wrote: > to -> of > > Rewrite to match the other resources (Title: Description). Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/async/async.d... sdk/lib/async/async.dart:70: * * [Using Future-Based APIs] On 2013/12/16 21:18:48, Kathy Walrath wrote: > Using -> Use Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/convert/conve... File sdk/lib/convert/convert.dart (right): https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/convert/conve... sdk/lib/convert/convert.dart:21: * Converters are often used in conjunction with streams On 2013/12/16 21:18:48, Kathy Walrath wrote: > in conjunction with -> with Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/convert/conve... sdk/lib/convert/convert.dart:22: * transforming the data that comes through the stream On 2013/12/16 21:18:48, Kathy Walrath wrote: > transforming -> to transform Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/convert/conve... sdk/lib/convert/convert.dart:42: * See the documentation for the [Codec] and the [Converter] classes On 2013/12/16 21:18:48, Kathy Walrath wrote: > and the -> and Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart File sdk/lib/io/io.dart (right): https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:21: * Consider it a pre-requisite to learn about Futures and Streams On 2013/12/16 21:18:48, Kathy Walrath wrote: > pre-requisite -> prerequisite > > "Consider it ..." seems weak. > > Actually, I'd just delete that sentence and most of the next. (just leave in the > mention of dart:async, and put the links to [Future] and [Stream] in the > previous sentence. > > And delete "import 'dart:async'". Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:33: * Because these methods On 2013/12/16 21:18:48, Kathy Walrath wrote: > Maybe: > > Because file system access can be slow, these methods are asynchronous and > return a Future. Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:33: * Because these methods On 2013/12/16 21:18:48, Kathy Walrath wrote: > Maybe: > > Because file system access can be slow, these methods are asynchronous and > return a Future. Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:48: * A [Directory] object represents a directory in the native file system. On 2013/12/16 21:18:48, Kathy Walrath wrote: > This is choppy. Maybe: > > Each instance of [File] or [Directory] represents a file or directory, > respectively, in the native file system. > Both File and Directory are subclasses of .... Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:50: * You can manipulate the file sytem through objects of these types. On 2013/12/16 21:18:48, Kathy Walrath wrote: > sytem! Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:86: * To read text synchronously from the command-line On 2013/12/16 21:18:48, Kathy Walrath wrote: > command-line -> command line > (since it's not an adjectival phrase modifying a noun) Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:86: * To read text synchronously from the command-line On 2013/12/16 21:18:48, Kathy Walrath wrote: > command-line -> command line > (since it's not an adjectival phrase modifying a noun) Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:91: * ## Other Resources On 2013/12/16 21:18:48, Kathy Walrath wrote: > Resources -> resources Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:99: * (https://www.dartlang.org/docs/tutorials/dartio/). On 2013/12/16 21:18:48, Kathy Walrath wrote: > I think the URL should be either dart-io or io. probably just io... Done. https://chromiumcodereview.appspot.com/111463004/diff/1/sdk/lib/io/io.dart#ne... sdk/lib/io/io.dart:99: * (https://www.dartlang.org/docs/tutorials/dartio/). On 2013/12/16 21:18:48, Kathy Walrath wrote: > I think the URL should be either dart-io or io. probably just io... Done.
Hi Søren: I've added some library-level docs to dart:async, dart:io, and dart:convert. Would you mind giving it a review? Thank you. mem
On 2013/12/16 22:24:09, mem wrote: Hi Søren: I just realized that I probably should've highlighted Platform in the library-level docs. The idea is to highlight the "most important" or "most often used" classes from the library and provide a tiny code snippet. If you see other classes that should be highlighted (without making the library-level docs overweighted) please let me know. Thanks. mem > Hi Søren: > I've added some library-level docs to dart:async, dart:io, and dart:convert. > > Would you mind giving it a review? > > Thank you. > mem
Adding Lasse as well (for dart:async and dart:convert) Sorry for the delay - it ended up in my personal email. https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/async/asy... File sdk/lib/async/async.dart (right): https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/async/asy... sdk/lib/async/async.dart:20: * waiting for a lengthy operation to complete. This is not really true in this example. As each Dart isolate is single-threaded delaying the calculation of fibonacci(50) using new Future(() => fibonacci(50)) will run the computation using Timer.run. When the timer fires for the computation the program will block for this computation to complete. For Dart code to really run in the background spawning an isolate is needed. For this intro I think it is better to use some dart:io and dart:html future as the stream examples below. I think the definition from the Future class is better "A Future is used to obtain a not yet available value, or error, sometime in the future.". It defines what it is (but not the numerous ways they can be created). https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/async/asy... sdk/lib/async/async.dart:25: * .then((fibValue) { print(fibValue); }) Use the => syntax everywhere? https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/async/asy... sdk/lib/async/async.dart:47: * stream.transform(UTF8.decoder).listen((data) { Shorten to stream.transform(UTF8.decoder).listen(print) ? https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/async/asy... sdk/lib/async/async.dart:51: * The stream returns a list of bytes. The stream emits a sequence of list of bytes (depending how the file reader provides its data). https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/async/asy... sdk/lib/async/async.dart:54: * to convert the file's UTF8-encoded text data into a Dart string. Maybe be more explicit on saying that the converter emits a sequence of strings. https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/convert/c... File sdk/lib/convert/convert.dart (right): https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/convert/c... sdk/lib/convert/convert.dart:6: * Converters for JSON and UTF-8, as well as support for creating additional I think the first line should be updated to something like this. Library for handling conversion between different data representations. It provides support for implementing converters in a way which makes them easy to chaining and work well with streams. It includes a number of converters including ones for JSON and UTF-8. Converters implemented using the https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/convert/c... sdk/lib/convert/convert.dart:16: * JSON is a simple text format for representing I think we should be a bit more precise about the JSON and UTF-8 converters. Say that JSON transforms between object structures strings using the JSON format, and that UTF-8 transfors between Strings and bytes. https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/core/date... File sdk/lib/core/date_time.dart (right): https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/core/date... sdk/lib/core/date_time.dart:8: * An instant in time, such as July 20, 1969, 8:18pm PDT. Maybe use GMT. https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/io/io.dart File sdk/lib/io/io.dart (right): https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/io/io.dar... sdk/lib/io/io.dart:21: * are in the [dart:async] library. are in -> are defined in https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/io/io.dar... sdk/lib/io/io.dart:25: * Given a pathname, you can create a [FileSystemEntity] to represent It is not possible to create a FileSystemEntity object directly. It is has not public constructors. It is an abstract superclass for File, Directory and Link. Maybe move this section below "File and Directory" and start with something like File, Directory and Link all extend FileSystemEntity. In addition to this FileSystemEntity has a number of static methods for working with paths. https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/io/io.dar... sdk/lib/io/io.dart:29: * Because file system access can be slow, these methods "can be slow" -> involved I/O. https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/io/io.dar... sdk/lib/io/io.dart:40: * ## File and Directory Mention Link as well? https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/io/io.dar... sdk/lib/io/io.dart:54: * ## HttpServer HttpServer -> HttpServer and HttpClient https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/io/io.dar... sdk/lib/io/io.dart:55: * The classes [HttpServer] and [HttpClient] provides HTTP server and HTTP client functionality. https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/io/io.dar... sdk/lib/io/io.dart:56: * To create an Http server, we recommend that you try using Start with: The [HttpServer] class provides the basic functionality for implementing an HTTP server. For some higher-level building-blocks, we recoment ... https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/io/io.dar... sdk/lib/io/io.dart:60: * in this library, makes it easy to provide content through HTTP servers. "easy to provide content through HTTP servers" -> "easier to implement HTTP servers" https://chromiumcodereview.appspot.com/111463004/diff/20001/sdk/lib/io/io.dar... sdk/lib/io/io.dart:61: * Shouldnt we have a sub-section for ## WebSocket ## Process ## Sockets To mention we have that. Otherwise people might miss that we support these areas as well.
And thanks for writing this top-level documentation.
lgtm https://codereview.chromium.org/111463004/diff/20001/sdk/lib/async/async.dart File sdk/lib/async/async.dart (right): https://codereview.chromium.org/111463004/diff/20001/sdk/lib/async/async.dart... sdk/lib/async/async.dart:20: * waiting for a lengthy operation to complete. Agree. I see a future as representing a computation that might not have completed yet. It doesn't provide functionality to do async stuff, it is just the result. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/async/async.dart... sdk/lib/async/async.dart:25: * .then((fibValue) { print(fibValue); }) Or not even that; in this case, you can just write .then(print) https://codereview.chromium.org/111463004/diff/20001/sdk/lib/async/async.dart... sdk/lib/async/async.dart:53: * Here, the code uses a UTF8 decoder (provided in the [dart:convert] library) Does [dart:convert] work as a DartDoc link? https://codereview.chromium.org/111463004/diff/20001/sdk/lib/convert/convert.... File sdk/lib/convert/convert.dart (right): https://codereview.chromium.org/111463004/diff/20001/sdk/lib/convert/convert.... sdk/lib/convert/convert.dart:9: * The dart:convert library works in both web apps and command-line apps. Wrap dart:convert in backquotes (`dart:convert`) for formatting? https://codereview.chromium.org/111463004/diff/20001/sdk/lib/convert/convert.... sdk/lib/convert/convert.dart:15: * [JsonCodec] and [Utf8Codec], named JSON and UTF8, respectively. Add empty line before "JSON" and before "UTF-8" below. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/convert/convert.... sdk/lib/convert/convert.dart:16: * JSON is a simple text format for representing Agree. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/core/date_time.dart File sdk/lib/core/date_time.dart (right): https://codereview.chromium.org/111463004/diff/20001/sdk/lib/core/date_time.d... sdk/lib/core/date_time.dart:8: * An instant in time, such as July 20, 1969, 8:18pm PDT. Good idea. Not everybody knows the national time zones of different countries (and even less the daylight savings zones).
Integrated changes based on your comments. I think I got everything. PTAL: I added 3 new sections to dart:io...Socket & ServerSocket, Process, and WebSocket. Thanks. mem https://codereview.chromium.org/111463004/diff/20001/sdk/lib/async/async.dart File sdk/lib/async/async.dart (right): https://codereview.chromium.org/111463004/diff/20001/sdk/lib/async/async.dart... sdk/lib/async/async.dart:20: * waiting for a lengthy operation to complete. On 2013/12/19 09:44:40, Søren Gjesse wrote: > This is not really true in this example. As each Dart isolate is single-threaded > delaying the calculation of fibonacci(50) using new Future(() => fibonacci(50)) > will run the computation using Timer.run. When the timer fires for the > computation the program will block for this computation to complete. > > For Dart code to really run in the background spawning an isolate is needed. For > this intro I think it is better to use some dart:io and dart:html future as the > stream examples below. > > I think the definition from the Future class is better "A Future is used to > obtain a not yet available value, or error, sometime in the future.". It defines > what it is (but not the numerous ways they can be created). Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/async/async.dart... sdk/lib/async/async.dart:47: * stream.transform(UTF8.decoder).listen((data) { On 2013/12/19 09:44:40, Søren Gjesse wrote: > Shorten to > > stream.transform(UTF8.decoder).listen(print) > > ? Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/async/async.dart... sdk/lib/async/async.dart:51: * The stream returns a list of bytes. On 2013/12/19 09:44:40, Søren Gjesse wrote: > The stream emits a sequence of list of bytes (depending how the file reader > provides its data). Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/async/async.dart... sdk/lib/async/async.dart:53: * Here, the code uses a UTF8 decoder (provided in the [dart:convert] library) On 2013/12/19 15:22:49, Lasse Reichstein Nielsen wrote: > Does [dart:convert] work as a DartDoc link? no but it should. I was optimistic. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/async/async.dart... sdk/lib/async/async.dart:54: * to convert the file's UTF8-encoded text data into a Dart string. On 2013/12/19 09:44:40, Søren Gjesse wrote: > Maybe be more explicit on saying that the converter emits a sequence of strings. Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/convert/convert.... File sdk/lib/convert/convert.dart (right): https://codereview.chromium.org/111463004/diff/20001/sdk/lib/convert/convert.... sdk/lib/convert/convert.dart:6: * Converters for JSON and UTF-8, as well as support for creating additional On 2013/12/19 09:44:40, Søren Gjesse wrote: > I think the first line should be updated to something like this. > > Library for handling conversion between different data representations. It > provides support for implementing converters in a way which makes them easy to > chaining and work well with streams. > > It includes a number of converters including ones for JSON and UTF-8. > > Converters implemented using the Your comment drifted off.... Anyway, the convention for the first line is that it's a one-liner and starts with a noun phrase. So I put in your changes with some alteration to stay within this constraint. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/convert/convert.... sdk/lib/convert/convert.dart:9: * The dart:convert library works in both web apps and command-line apps. On 2013/12/19 15:22:49, Lasse Reichstein Nielsen wrote: > Wrap dart:convert in backquotes (`dart:convert`) for formatting? Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/convert/convert.... sdk/lib/convert/convert.dart:15: * [JsonCodec] and [Utf8Codec], named JSON and UTF8, respectively. On 2013/12/19 15:22:49, Lasse Reichstein Nielsen wrote: > Add empty line before "JSON" and before "UTF-8" below. Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/convert/convert.... sdk/lib/convert/convert.dart:16: * JSON is a simple text format for representing On 2013/12/19 09:44:40, Søren Gjesse wrote: > I think we should be a bit more precise about the JSON and UTF-8 converters. Say > that JSON transforms between object structures strings using the JSON format, > and that UTF-8 transfors between Strings and bytes. Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/core/date_time.dart File sdk/lib/core/date_time.dart (right): https://codereview.chromium.org/111463004/diff/20001/sdk/lib/core/date_time.d... sdk/lib/core/date_time.dart:8: * An instant in time, such as July 20, 1969, 8:18pm PDT. On 2013/12/19 15:22:49, Lasse Reichstein Nielsen wrote: > Good idea. Not everybody knows the national time zones of different countries > (and even less the daylight savings zones). Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart File sdk/lib/io/io.dart (right): https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:21: * are in the [dart:async] library. On 2013/12/19 09:44:40, Søren Gjesse wrote: > are in -> are defined in Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:25: * Given a pathname, you can create a [FileSystemEntity] to represent On 2013/12/19 09:44:40, Søren Gjesse wrote: > It is not possible to create a FileSystemEntity object directly. It is has not > public constructors. It is an abstract superclass for File, Directory and Link. > > Maybe move this section below "File and Directory" and start with something > like > > File, Directory and Link all extend FileSystemEntity. In addition to this > FileSystemEntity has a number of static methods for working with paths. Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:29: * Because file system access can be slow, these methods On 2013/12/19 09:44:40, Søren Gjesse wrote: > "can be slow" -> involved I/O. Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:40: * ## File and Directory On 2013/12/19 09:44:40, Søren Gjesse wrote: > Mention Link as well? Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:54: * ## HttpServer On 2013/12/19 09:44:40, Søren Gjesse wrote: > HttpServer -> HttpServer and HttpClient Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:55: * On 2013/12/19 09:44:40, Søren Gjesse wrote: > The classes [HttpServer] and [HttpClient] provides HTTP server and HTTP client > functionality. Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:56: * To create an Http server, we recommend that you try using On 2013/12/19 09:44:40, Søren Gjesse wrote: > Start with: > > The [HttpServer] class provides the basic functionality for implementing an HTTP > server. For some higher-level building-blocks, we recoment ... Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:60: * in this library, makes it easy to provide content through HTTP servers. On 2013/12/19 09:44:40, Søren Gjesse wrote: > "easy to provide content through HTTP servers" -> "easier to implement HTTP > servers" Done. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:61: * Thanks. It's great to have a more complete list of important classes in this library. On 2013/12/19 09:44:40, Søren Gjesse wrote: > Shouldnt we have a sub-section for > > ## WebSocket > ## Process > ## Sockets > > To mention we have that. Otherwise people might miss that we support these areas > as well. https://codereview.chromium.org/111463004/diff/20001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:61: * On 2013/12/19 09:44:40, Søren Gjesse wrote: > Shouldnt we have a sub-section for > > ## WebSocket > ## Process > ## Sockets > > To mention we have that. Otherwise people might miss that we support these areas > as well. Done.
I just realized the code sample that I used for WebSocket is the client side and uses the HTML class. So I need to fix that. mem
Fixed the WebSocket text and sample. It just use the dart:io API now. PTAL.
LGTM, with comments. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart File sdk/lib/io/io.dart (right): https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:87: * you cannot call `exitCode`. Change the paragraph above to: Using `start()` returns a future which completes with a [Process] object when the process has started. This [Process] object allows you to interact with the process while it is running. Using `run()` returns a future which completes with a [ProcessResult] object when the spawend process has terminated. This [ProcessResult] object collects the output and exit code from the process. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:96: * client and server applications to send data back and forth. Consider changing the first sentence to: The [WebSocket] class provides support for the web socket protocol. This allows full-duplex communications between client and server applications. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:97: * Use the WebSocket class in the dart:html library for web clients. Add `s around dart:html. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:101: * (the URI ends with '/ws'), Consider changing the text above to: A web socket server uses a normal HTTP server for accepting web socket connections. The initial handshake is a HTTP request which is then upgraded to a web socket connection. and start a new sentence below with 'The serer upgrades ...' https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:138: * The server creates a socket using the `bind()` method and then listens for data 'creates a socket' -> 'creates a listening socket' 'listens for data on the socket' -> 'listens for incoming connections on the socket' https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:148: * A client connects to a Socket using the `connect()` method, 'connects to a Socket' -> 'connects a Socket' https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:157: * Maybe add a short paragraph: Besides [Socket] and [ServerSocket] there are classes [RawSocket] and [RawServerSocket] are available for more low-level access to async socket IO.
fixed issues from Søren. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart File sdk/lib/io/io.dart (right): https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:87: * you cannot call `exitCode`. On 2014/01/02 09:49:18, Søren Gjesse wrote: > Change the paragraph above to: > > Using `start()` returns a future which completes with a [Process] object when > the process has started. This [Process] object allows you to interact with the > process while it is running. Using `run()` returns a future which completes with > a [ProcessResult] object when the spawend process has terminated. This > [ProcessResult] object collects the output and exit code from the process. Done. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:96: * client and server applications to send data back and forth. On 2014/01/02 09:49:18, Søren Gjesse wrote: > Consider changing the first sentence to: > > The [WebSocket] class provides support for the web socket protocol. This allows > full-duplex communications between client and server applications. Done. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:97: * Use the WebSocket class in the dart:html library for web clients. On 2014/01/02 09:49:18, Søren Gjesse wrote: > Add `s around dart:html. Done. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:101: * (the URI ends with '/ws'), On 2014/01/02 09:49:18, Søren Gjesse wrote: > Consider changing the text above to: > > A web socket server uses a normal HTTP server for accepting web socket > connections. The initial handshake is a HTTP request which is then upgraded to a > web socket connection. > > and start a new sentence below with 'The serer upgrades ...' Done. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:138: * The server creates a socket using the `bind()` method and then listens for data On 2014/01/02 09:49:18, Søren Gjesse wrote: > 'creates a socket' -> 'creates a listening socket' > > 'listens for data on the socket' -> 'listens for incoming connections on the > socket' Done. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:148: * A client connects to a Socket using the `connect()` method, On 2014/01/02 09:49:18, Søren Gjesse wrote: > 'connects to a Socket' -> 'connects a Socket' Done. https://codereview.chromium.org/111463004/diff/50001/sdk/lib/io/io.dart#newco... sdk/lib/io/io.dart:157: * On 2014/01/02 09:49:18, Søren Gjesse wrote: > Maybe add a short paragraph: > > Besides [Socket] and [ServerSocket] there are classes [RawSocket] and > [RawServerSocket] are available for more low-level access to async socket IO. Done.
Message was sent while issue was closed.
Committed patchset #6 manually as r31421 (presubmit successful). |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
