Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
210393005:
update API docs for HTTP client and server-related classes (Closed)
|
Created:
6 years, 9 months ago by mem Modified:
6 years, 8 months ago CC:
reviews_dartlang.org Visibility:
Public. |
Descriptionupdate API docs for HTTP client and server-related classes
BUG=
R=ajohnsen@google.com, sgjesse@google.com
Committed: https://code.google.com/p/dart/source/detail?r=34524
Patch Set 1 #
Total comments: 46
Patch Set 2 : incorporated comments from Søren #
Total comments: 34
Patch Set 3 : incorporated feedback from Søren and Anders #Patch Set 4 : merge with master #Patch Set 5 : merge with master #
Messages
Total messages: 7 (0 generated)
Hi Søren: I've edited the API docs for the client/server Http* classes. Thanks for your time reviewing them. mem
https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart File sdk/lib/io/http.dart (right): https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:66: * Port 80 is generally already in use, so your servers should use a Thin this sentence should be something like this: Port 80 is the default HTTP port. However on most systems accessing this requires super-user privileges. For local testing consider using a non-reserved port (above 1023). https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:93: * Use [bindSecure] to create a secure HTTPS connection. 'create a secure HTTPS connection' -> 'create a HTTPS server'. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:94: * The client needs to present a value certificate to connect successfully. The client does not need to present a certificate unless the optional argument requestClientCertificate is set to true. I dont we should talk about client certificates here. It is the server that needs a certificate to present to the client. This is the certificate named 'localhost_cert' in the certificate database found in the 'pkcert' directory. This certificate database is managed using the Mozilla certutil tool (see https://developer.mozilla.org/en-US/docs/NSS/tools/NSS_Tools_certutil). We should mention that we use the NSS library to handle SSL and that the Mozilla certutil must be used to manipulate the certificate database. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:108: * 80, Use the default HTTPS port 443 here. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:109: * backlog: 5, Just remove the backlog optional argument in the sample. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:112: * ReceivePort serverPort = new ReceivePort(); Please remove the receive port. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:123: * a [ServerSocket]. By subclassing ServerSocket you can provide I don't think we should mention "subclassing ServerSocket" - do we have a use-case for that? https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:338: * request.headers.add(HttpHeaders.CONTENT_ENCODING, 'JSON'); We should probably use a more common header, e.g. HttpHeaders.CONTENT_TYPE or HttpHeaders.CACHE_CONTROL, e.g. request.headers.set(HttpHeaders.CACHE_CONTROL, 'max-age=3600, must-revalidate'); also use 'set' instead of 'add'. For most headers setting a single value is the right thing to do. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:342: * print(request.headers.value(HttpHeaders.FROM)); I think FROM is not the best choice here - don't think it is set very often. How about USER_AGENT instead? https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:345: * request.headers['FROM'] = john@example.com' The operator[] is only for getting the list of values, e.g. print(request.headers['FROM']) however using this always returns a list of values. I think some additional information on the HttpHeaders object is needed. In that is holds a list of values for each name as the standard allows. In most cases a name only holds a single value, so setting using 'set' and retreive using value() is the most common mode of operation. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:857: * An HttpRequest object contains an [HttpResponse] object, 'contains an' provides access to the associated HttpResponse object through the response property. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:972: * Every HttpRequest object contains an HttpResponse object, See above. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1099: * to an [HttpServer] and receive an [HttpClientResponse] back. [HttpServer] should just be HTTP server, as it can be any server, not just one implemented in dart:io. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1106: * When the first future completes with a HttpClientRequest, the underlying `` or [] around HttpClientRequest. I cannot remember when [] provides a functional link - is it only for arguments? https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1114: * an [HttpClientResponse] object. Maybe `` instead of []. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1116: * When the HTTP response is received from the server, This seems to be duplicate of the paragraph above. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1125: * client.getUrl(Uri.parse("http://www.example.com/")) Let's stick to the 4-space indent specified by the style guide. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1447: * To set up a request, set the headers using one of the many properties The headers is not set using 'one of the many properties provided in this class' but through the headers property. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1449: * HttpClientRequest is an [IOSink]. Use one of the methods from IOSink, Remove 'one of' as you can use several in combination. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1565: * new HttpClient().post('localhost', 80, '/file.txt') Use get here to avoid data in the request. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1566: * .then( ... ) Change .then( ... ) to .then((HttpClientRequest request) => request.close()) https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/socket.dart File sdk/lib/io/socket.dart (right): https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/socket.dar... sdk/lib/io/socket.dart:44: * A remote or local internet address. Maybe change this to something like this This object holds an internet address. If this internet address is the result of a DNS lookup the address also hold the hostname used to make the lookup.
Hi Søren: I incorporated your comments. Thanks for them. mem https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart File sdk/lib/io/http.dart (right): https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:66: * Port 80 is generally already in use, so your servers should use a On 2014/03/25 10:27:26, Søren Gjesse wrote: > Thin this sentence should be something like this: > > Port 80 is the default HTTP port. However on most systems accessing this > requires super-user privileges. For local testing consider using a non-reserved > port (above 1023). Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:93: * Use [bindSecure] to create a secure HTTPS connection. On 2014/03/25 10:27:26, Søren Gjesse wrote: > 'create a secure HTTPS connection' -> 'create a HTTPS server'. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:94: * The client needs to present a value certificate to connect successfully. On 2014/03/25 10:27:26, Søren Gjesse wrote: > The client does not need to present a certificate unless the optional argument > requestClientCertificate is set to true. I dont we should talk about client > certificates here. > > It is the server that needs a certificate to present to the client. This is the > certificate named 'localhost_cert' in the certificate database found in the > 'pkcert' directory. > > This certificate database is managed using the Mozilla certutil tool (see > https://developer.mozilla.org/en-US/docs/NSS/tools/NSS_Tools_certutil). We > should mention that we use the NSS library to handle SSL and that the Mozilla > certutil must be used to manipulate the certificate database. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:108: * 80, On 2014/03/25 10:27:26, Søren Gjesse wrote: > Use the default HTTPS port 443 here. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:109: * backlog: 5, On 2014/03/25 10:27:26, Søren Gjesse wrote: > Just remove the backlog optional argument in the sample. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:112: * ReceivePort serverPort = new ReceivePort(); On 2014/03/25 10:27:26, Søren Gjesse wrote: > Please remove the receive port. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:123: * a [ServerSocket]. By subclassing ServerSocket you can provide On 2014/03/25 10:27:26, Søren Gjesse wrote: > I don't think we should mention "subclassing ServerSocket" - do we have a > use-case for that? Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:342: * print(request.headers.value(HttpHeaders.FROM)); On 2014/03/25 10:27:26, Søren Gjesse wrote: > I think FROM is not the best choice here - don't think it is set very often. How > about USER_AGENT instead? Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:345: * request.headers['FROM'] = john@example.com' On 2014/03/25 10:27:26, Søren Gjesse wrote: > The operator[] is only for getting the list of values, e.g. > > print(request.headers['FROM']) > > however using this always returns a list of values. > > I think some additional information on the HttpHeaders object is needed. In that > is holds a list of values for each name as the standard allows. In most cases a > name only holds a single value, so setting using 'set' and retreive using > value() is the most common mode of operation. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:345: * request.headers['FROM'] = john@example.com' On 2014/03/25 10:27:26, Søren Gjesse wrote: > The operator[] is only for getting the list of values, e.g. > > print(request.headers['FROM']) > > however using this always returns a list of values. > > I think some additional information on the HttpHeaders object is needed. In that > is holds a list of values for each name as the standard allows. In most cases a > name only holds a single value, so setting using 'set' and retreive using > value() is the most common mode of operation. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:857: * An HttpRequest object contains an [HttpResponse] object, On 2014/03/25 10:27:26, Søren Gjesse wrote: > 'contains an' provides access to the associated HttpResponse object through the > response property. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:972: * Every HttpRequest object contains an HttpResponse object, On 2014/03/25 10:27:26, Søren Gjesse wrote: > See above. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1099: * to an [HttpServer] and receive an [HttpClientResponse] back. On 2014/03/25 10:27:26, Søren Gjesse wrote: > [HttpServer] should just be HTTP server, as it can be any server, not just one > implemented in dart:io. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1099: * to an [HttpServer] and receive an [HttpClientResponse] back. On 2014/03/25 10:27:26, Søren Gjesse wrote: > [HttpServer] should just be HTTP server, as it can be any server, not just one > implemented in dart:io. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1106: * When the first future completes with a HttpClientRequest, the underlying On 2014/03/25 10:27:26, Søren Gjesse wrote: > `` or [] around HttpClientRequest. I cannot remember when [] provides a > functional link - is it only for arguments? Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1114: * an [HttpClientResponse] object. On 2014/03/25 10:27:26, Søren Gjesse wrote: > Maybe `` instead of []. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1116: * When the HTTP response is received from the server, On 2014/03/25 10:27:26, Søren Gjesse wrote: > This seems to be duplicate of the paragraph above. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1125: * client.getUrl(Uri.parse("http://www.example.com/")) On 2014/03/25 10:27:26, Søren Gjesse wrote: > Let's stick to the 4-space indent specified by the style guide. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1447: * To set up a request, set the headers using one of the many properties On 2014/03/25 10:27:26, Søren Gjesse wrote: > The headers is not set using 'one of the many properties provided in this class' > but through the headers property. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1449: * HttpClientRequest is an [IOSink]. Use one of the methods from IOSink, On 2014/03/25 10:27:26, Søren Gjesse wrote: > Remove 'one of' as you can use several in combination. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1565: * new HttpClient().post('localhost', 80, '/file.txt') On 2014/03/25 10:27:26, Søren Gjesse wrote: > Use get here to avoid data in the request. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1565: * new HttpClient().post('localhost', 80, '/file.txt') On 2014/03/25 10:27:26, Søren Gjesse wrote: > Use get here to avoid data in the request. Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/http.dart#... sdk/lib/io/http.dart:1566: * .then( ... ) On 2014/03/25 10:27:26, Søren Gjesse wrote: > Change > > .then( ... ) > > to > > .then((HttpClientRequest request) => request.close()) Done. https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/socket.dart File sdk/lib/io/socket.dart (right): https://chromiumcodereview.appspot.com/210393005/diff/1/sdk/lib/io/socket.dar... sdk/lib/io/socket.dart:44: * A remote or local internet address. We need to use noun phrases for the short one line descriptions. On 2014/03/25 10:27:26, Søren Gjesse wrote: > Maybe change this to something like this > > This object holds an internet address. If this internet address is the result of > a DNS lookup the address also hold the hostname used to make the lookup.
LGTM, but let Anders have a look as well. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.dart File sdk/lib/io/http.dart (right): https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:336: * In some situations the headers are immutable, We could go into the details on when headers are immutable 1. HttpRequest and HttpClientResponse always have immutable headers 2. HttpResponse and HttpClientRequest have immutable headers from when the body is written to. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1136: * // Prepare the request, then call close to send it. Maybe we should say what we mean with 'prepare the request'. Which is optionally add headers and add a body. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1156: * `Accept-Encoding` header to something else. Add: To just turn off gzip compression of the response just clear this header: request.headers.removeAll(HttpHeaders.ACCEPT_ENCODING) https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1158: * ## Closing the connection connection -> HttpClient
LGTM, very awesome :) https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.dart File sdk/lib/io/http.dart (right): https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:68: * using a non-reserved port (above 1023). 1023 is a funny number :) Maybe say 1024 and above? https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:98: * the database found in the `pkcert` directory . after directory. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:99: * This certificate is used by Dart for testing purposes. I don't think we need this line. It should be obvious to the reader, that examples are guidelines, not real-world deployable scripts. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:124: * Dart uses the NSS library to handle SSL and Mozilla I'm reading the 'and' wrong here. Would this be better? Dart uses the NSS library to handle SSL, and the Mozilla certutil must be used to manipulate the certificate database. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:127: * ## Connect to a socket a server socket https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:147: * about the streaming qualities of an HttpServer. We could consider to specify, that pausing the subscription of the stream, will apply pausing on the OS level (it's a good thing :). https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:979: * An [HttpResponse] contains the headers and data returned contains is a dangerous words, as it makes it sound like it literally holds on to the data. That's not the case - it's written to the socket when available. I'm not sure what it should say though... :/ https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:985: * body of the HttpResponse object. there is no 'body' object. maybe just say writin to the HttpResponse object As the API is HttpResponse.write https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:988: * header of the response. This class implements [IOSink]. headers https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:988: * header of the response. This class implements [IOSink]. Maybe move IOSink to the paragraph above, as it's the interface that's used to write to the HttpResponse. Or better yet, move the first part of this paragraph below the next ones. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1020: * `write()` method being used takes a string parameter. This sentence is weird. write can only be called with a string. Maybe say something like: An exception is thrown if you use the 'write' method while an unsupported content-type is set. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1123: * request to the server. We send data as soon as the first either write or close. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/socket... File sdk/lib/io/socket.dart (right): https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/socket... sdk/lib/io/socket.dart:50: * endpoint to which a socket can connect or a listening socket can We call the listening socket a server socket in the APIs.
Incorporated all comments. Thanks for your help. So that was an "LGTM with comments" and now that I've fixed the comments, I can push this, right? I pushed something prematurely before and don't want to make the same mistake. Thanks. mem https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.dart File sdk/lib/io/http.dart (right): https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:68: * using a non-reserved port (above 1023). On 2014/03/27 12:29:36, Anders Johnsen wrote: > 1023 is a funny number :) Maybe say 1024 and above? Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:98: * the database found in the `pkcert` directory On 2014/03/27 12:29:36, Anders Johnsen wrote: > . after directory. Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:99: * This certificate is used by Dart for testing purposes. On 2014/03/27 12:29:36, Anders Johnsen wrote: > I don't think we need this line. It should be obvious to the reader, that > examples are guidelines, not real-world deployable scripts. Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:124: * Dart uses the NSS library to handle SSL and Mozilla On 2014/03/27 12:29:36, Anders Johnsen wrote: > I'm reading the 'and' wrong here. Would this be better? > > Dart uses the NSS library to handle SSL, and the Mozilla certutil must be used > to manipulate the certificate database. Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:127: * ## Connect to a socket On 2014/03/27 12:29:36, Anders Johnsen wrote: > a server socket Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:147: * about the streaming qualities of an HttpServer. On 2014/03/27 12:29:36, Anders Johnsen wrote: > We could consider to specify, that pausing the subscription of the stream, will > apply pausing on the OS level (it's a good thing :). Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:336: * In some situations the headers are immutable, On 2014/03/27 10:09:55, Søren Gjesse wrote: > We could go into the details on when headers are immutable > > 1. HttpRequest and HttpClientResponse always have immutable headers > 2. HttpResponse and HttpClientRequest have immutable headers from > when the body is written to. Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:979: * An [HttpResponse] contains the headers and data returned On 2014/03/27 12:29:36, Anders Johnsen wrote: > contains is a dangerous words, as it makes it sound like it literally holds on > to the data. That's not the case - it's written to the socket when available. > > I'm not sure what it should say though... :/ Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:985: * body of the HttpResponse object. On 2014/03/27 12:29:36, Anders Johnsen wrote: > there is no 'body' object. maybe just say > > writin to the HttpResponse object > > As the API is HttpResponse.write Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:988: * header of the response. This class implements [IOSink]. On 2014/03/27 12:29:36, Anders Johnsen wrote: > headers Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:988: * header of the response. This class implements [IOSink]. On 2014/03/27 12:29:36, Anders Johnsen wrote: > Maybe move IOSink to the paragraph above, as it's the interface that's used to > write to the HttpResponse. > > Or better yet, move the first part of this paragraph below the next ones. Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1020: * `write()` method being used takes a string parameter. On 2014/03/27 12:29:36, Anders Johnsen wrote: > This sentence is weird. write can only be called with a string. > > Maybe say something like: > > An exception is thrown if you use the 'write' method while an unsupported > content-type is set. Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1020: * `write()` method being used takes a string parameter. On 2014/03/27 12:29:36, Anders Johnsen wrote: > This sentence is weird. write can only be called with a string. > > Maybe say something like: > > An exception is thrown if you use the 'write' method while an unsupported > content-type is set. Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1123: * request to the server. On 2014/03/27 12:29:36, Anders Johnsen wrote: > We send data as soon as the first either write or close. Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1136: * // Prepare the request, then call close to send it. On 2014/03/27 10:09:55, Søren Gjesse wrote: > Maybe we should say what we mean with 'prepare the request'. Which is optionally > add headers and add a body. Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1156: * `Accept-Encoding` header to something else. On 2014/03/27 10:09:55, Søren Gjesse wrote: > Add: > > To just turn off gzip compression of the response just clear this header: > > request.headers.removeAll(HttpHeaders.ACCEPT_ENCODING) Done. https://chromiumcodereview.appspot.com/210393005/diff/20001/sdk/lib/io/http.d... sdk/lib/io/http.dart:1158: * ## Closing the connection On 2014/03/27 10:09:55, Søren Gjesse wrote: > connection -> HttpClient Done.
Message was sent while issue was closed.
Committed patchset #5 manually as r34524 (presubmit successful). |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||
