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

Issue 9500002: Rename blahHandler to onBlah throughout dart:io. (Closed)

Created:
8 years, 9 months ago by Mads Ager (google)
Modified:
8 years, 9 months ago
Reviewers:
Søren Gjesse
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Rename blahHandler to onBlah throughout dart:io. Additionally, change all single-shot methods to take their callback as an argument instead of registering it on the object. I have attempted to find all uses in the repo but there might be some firefighting to do when this lands. R=sgjesse@google.com BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=4784

Patch Set 1 #

Patch Set 2 : Revert temporary edit #

Total comments: 20

Patch Set 3 : Make single-shot functions take their callback directly #

Patch Set 4 : Renaming #

Patch Set 5 : Renaming #

Patch Set 6 : Renaming #

Total comments: 8

Patch Set 7 : Address comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1172 lines, -1517 lines) Patch
M frog/leg/tools/mini_parser.dart View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M frog/server/frog_server.dart View 1 2 2 chunks +4 lines, -4 lines 0 comments Download
M frog/tests/frog/src/FrogServerTest.dart View 1 chunk +1 line, -1 line 0 comments Download
M runtime/bin/chunked_stream.dart View 1 2 4 chunks +10 lines, -10 lines 0 comments Download
M runtime/bin/directory.dart View 1 2 3 4 5 6 6 chunks +48 lines, -61 lines 0 comments Download
M runtime/bin/directory_impl.dart View 1 2 3 4 5 6 10 chunks +52 lines, -61 lines 0 comments Download
M runtime/bin/file.dart View 1 2 3 23 chunks +67 lines, -172 lines 0 comments Download
M runtime/bin/file_impl.dart View 1 2 3 4 5 6 48 chunks +147 lines, -253 lines 0 comments Download
M runtime/bin/http.dart View 1 2 3 4 5 6 4 chunks +14 lines, -14 lines 0 comments Download
M runtime/bin/http_impl.dart View 1 2 3 4 5 6 20 chunks +104 lines, -104 lines 0 comments Download
M runtime/bin/input_stream.dart View 1 2 4 chunks +13 lines, -13 lines 0 comments Download
M runtime/bin/list_stream_impl.dart View 1 2 3 1 chunk +3 lines, -3 lines 0 comments Download
M runtime/bin/output_stream.dart View 1 2 3 3 chunks +5 lines, -5 lines 0 comments Download
M runtime/bin/process.dart View 2 chunks +16 lines, -16 lines 0 comments Download
M runtime/bin/process_impl.dart View 9 chunks +21 lines, -22 lines 0 comments Download
M runtime/bin/socket.dart View 1 2 4 chunks +9 lines, -9 lines 0 comments Download
M runtime/bin/socket_impl.dart View 1 2 7 chunks +18 lines, -18 lines 0 comments Download
M runtime/bin/socket_stream_impl.dart View 1 2 3 7 chunks +24 lines, -24 lines 0 comments Download
M runtime/bin/stream_util.dart View 1 2 3 3 chunks +10 lines, -10 lines 0 comments Download
M runtime/bin/string_stream.dart View 1 2 4 chunks +13 lines, -13 lines 0 comments Download
M samples/chat/chat_server_lib.dart View 1 2 5 chunks +9 lines, -9 lines 0 comments Download
M samples/chat/chat_stress_client.dart View 1 2 4 chunks +16 lines, -16 lines 0 comments Download
M samples/tests/samples/src/chat/ChatServerTest.dart View 1 2 4 chunks +16 lines, -16 lines 0 comments Download
M samples/total/server/DartCompiler.dart View 1 chunk +3 lines, -3 lines 0 comments Download
M samples/total/server/TotalRunner.dart View 1 chunk +3 lines, -3 lines 0 comments Download
M tests/standalone/src/ChunkedStreamTest.dart View 1 2 2 chunks +4 lines, -4 lines 0 comments Download
M tests/standalone/src/DartStdIOPipeTest.dart View 1 2 3 4 5 6 4 chunks +5 lines, -5 lines 0 comments Download
M tests/standalone/src/DirectoryInvalidArgumentsTest.dart View 2 chunks +3 lines, -3 lines 0 comments Download
M tests/standalone/src/DirectoryTest.dart View 1 2 3 4 5 6 11 chunks +92 lines, -119 lines 0 comments Download
M tests/standalone/src/EchoServerStreamTest.dart View 1 2 5 chunks +8 lines, -8 lines 0 comments Download
M tests/standalone/src/EchoServerTest.dart View 1 2 6 chunks +11 lines, -11 lines 0 comments Download
M tests/standalone/src/FileInputStreamTest.dart View 1 2 5 chunks +8 lines, -8 lines 0 comments Download
M tests/standalone/src/FileInvalidArgumentsTest.dart View 1 2 3 8 chunks +33 lines, -41 lines 0 comments Download
M tests/standalone/src/FileOutputStreamTest.dart View 1 2 3 4 5 6 3 chunks +9 lines, -12 lines 0 comments Download
M tests/standalone/src/FileTest.dart View 1 2 3 4 5 6 27 chunks +204 lines, -276 lines 0 comments Download
M tests/standalone/src/HttpTest.dart View 1 2 3 4 5 6 11 chunks +20 lines, -20 lines 0 comments Download
M tests/standalone/src/ListInputStreamTest.dart View 1 2 13 chunks +32 lines, -32 lines 0 comments Download
M tests/standalone/src/ListOutputStreamTest.dart View 1 2 3 5 chunks +9 lines, -9 lines 0 comments Download
M tests/standalone/src/ProcessBrokenPipeTest.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/standalone/src/ProcessExitNegativeTest.dart View 2 chunks +2 lines, -2 lines 0 comments Download
M tests/standalone/src/ProcessExitTest.dart View 2 chunks +2 lines, -2 lines 0 comments Download
M tests/standalone/src/ProcessSegfaultTest.dart View 2 chunks +2 lines, -2 lines 0 comments Download
M tests/standalone/src/ProcessStartExceptionTest.dart View 2 chunks +3 lines, -3 lines 0 comments Download
M tests/standalone/src/ProcessStdIOScript.dart View 2 chunks +4 lines, -4 lines 0 comments Download
M tests/standalone/src/ProcessStderrTest.dart View 3 chunks +4 lines, -4 lines 0 comments Download
M tests/standalone/src/ProcessStdoutTest.dart View 3 chunks +4 lines, -4 lines 0 comments Download
M tests/standalone/src/ProcessWorkingDirectoryTest.dart View 3 chunks +5 lines, -5 lines 0 comments Download
M tests/standalone/src/SocketCloseTest.dart View 1 2 4 chunks +9 lines, -9 lines 0 comments Download
M tests/standalone/src/SocketManyConnectionsTest.dart View 1 2 3 chunks +4 lines, -4 lines 0 comments Download
M tests/standalone/src/SocketStreamCloseTest.dart View 1 2 3 9 chunks +14 lines, -14 lines 0 comments Download
M tests/standalone/src/StreamPipeTest.dart View 1 2 7 chunks +10 lines, -10 lines 0 comments Download
M tests/standalone/src/StringStreamTest.dart View 1 2 5 chunks +7 lines, -7 lines 0 comments Download
M tests/standalone/src/TestingServer.dart View 2 chunks +3 lines, -3 lines 0 comments Download
M tools/testing/dart/status_file_parser.dart View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M tools/testing/dart/test_runner.dart View 1 2 3 4 5 6 8 chunks +16 lines, -16 lines 0 comments Download
M tools/testing/dart/test_suite.dart View 1 2 3 4 5 6 4 chunks +14 lines, -15 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Mads Ager (google)
8 years, 9 months ago (2012-02-28 17:17:49 UTC) #1
Mads Ager (google)
The stable test binaries will have to be updated as part of this as well. ...
8 years, 9 months ago (2012-02-28 17:33:33 UTC) #2
Søren Gjesse
I think we should discuss some of the names. Of cause it is easy to ...
8 years, 9 months ago (2012-02-29 07:41:24 UTC) #3
Mads Ager (google)
I agree. I'll do a sweep through all the onBlah names now and update.
8 years, 9 months ago (2012-02-29 08:15:41 UTC) #4
Mads Ager (google)
Having looking into this some more I think this awkwardness in naming the event handlers ...
8 years, 9 months ago (2012-02-29 11:16:52 UTC) #5
Mads Ager (google)
Changed all one-shot methods to take their callback as an argument. Directory listing should be ...
8 years, 9 months ago (2012-02-29 14:57:32 UTC) #6
Søren Gjesse
LGTM! https://chromiumcodereview.appspot.com/9500002/diff/8007/runtime/bin/directory.dart File runtime/bin/directory.dart (right): https://chromiumcodereview.appspot.com/9500002/diff/8007/runtime/bin/directory.dart#newcode53 runtime/bin/directory.dart:53: void createTemp(void callback()); Maybe we should consider changing ...
8 years, 9 months ago (2012-02-29 15:42:20 UTC) #7
Mads Ager (google)
8 years, 9 months ago (2012-03-06 07:05:13 UTC) #8
http://codereview.chromium.org/9500002/diff/8007/runtime/bin/directory_impl.dart
File runtime/bin/directory_impl.dart (right):

http://codereview.chromium.org/9500002/diff/8007/runtime/bin/directory_impl.d...
runtime/bin/directory_impl.dart:89: _onError("Could not create temporary
directory [$_path]: " +
On 2012/02/29 15:42:20, Søren Gjesse wrote:
> Indentation.

Done.

http://codereview.chromium.org/9500002/diff/8007/runtime/bin/directory_impl.d...
runtime/bin/directory_impl.dart:108: void delete(bool recursive, void
callback()) {
On 2012/02/29 15:42:20, Søren Gjesse wrote:
> We should consider revising this to not take a boolean argument writing this
to
> delete a directory looks a bit strange:
> 
> d.delete(false, () => print("done"))
> d.delete(true, () => print("done"))
> 
> What is a "false" deletion of a directory?
> 
> How about
> 
> 
> d.delete(() => print("done"))
> d.deleteRecursive(() => print("done"))
> 
> or
> 
> 
> d.delete(DeleteMode.NON_RECURSIVE, () => print("done"))
> d.delete(DeleteMode.RECURSIVE, () => print("done"))

I have gone for two separate methods here. delete and deleteRecursively.

http://codereview.chromium.org/9500002/diff/8007/tests/standalone/src/Process...
File tests/standalone/src/ProcessTestUtil.dart (right):

http://codereview.chromium.org/9500002/diff/8007/tests/standalone/src/Process...
tests/standalone/src/ProcessTestUtil.dart:1: // Copyright (c) 2012, the Dart
project authors.  Please see the AUTHORS file
On 2012/02/29 15:42:20, Søren Gjesse wrote:
> No need to change this.

Done.

Powered by Google App Engine
This is Rietveld 408576698