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

Issue 8883017: Split File into File and RandomAccessFile. (Closed)

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

Description

Split File into File and RandomAccessFile. Update all users of the API. R=sgjesse@google.com BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=2272

Patch Set 1 #

Patch Set 2 : Minor updates. #

Total comments: 6

Patch Set 3 : Address review comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+392 lines, -387 lines) Patch
M frog/file_system_vm.dart View 1 chunk +1 line, -2 lines 0 comments Download
M frog/leg/scanner/vm_scanner_bench.dart View 1 chunk +1 line, -2 lines 0 comments Download
M frog/leg/tools/mini_parser.dart View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M runtime/bin/file.dart View 1 2 3 chunks +50 lines, -36 lines 0 comments Download
M runtime/bin/file_impl.dart View 1 2 40 chunks +183 lines, -156 lines 0 comments Download
M samples/chat/chat_server_lib.dart View 1 2 chunks +21 lines, -10 lines 0 comments Download
M tests/standalone/src/DirectoryTest.dart View 2 chunks +33 lines, -36 lines 0 comments Download
M tests/standalone/src/FileInputStreamTest.dart View 5 chunks +1 line, -9 lines 0 comments Download
M tests/standalone/src/FileInvalidArgumentsTest.dart View 4 chunks +3 lines, -26 lines 0 comments Download
M tests/standalone/src/FileTest.dart View 31 chunks +70 lines, -77 lines 0 comments Download
M tools/testing/dart/multitest.dart View 2 chunks +26 lines, -27 lines 0 comments Download
M tools/testing/dart/test_suite.dart View 1 chunk +1 line, -2 lines 0 comments Download
M utils/markdown/markdown.dart View 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Mads Ager (google)
9 years ago (2011-12-08 14:27:07 UTC) #1
Søren Gjesse
lgtm http://codereview.chromium.org/8883017/diff/2001/runtime/bin/file.dart File runtime/bin/file.dart (right): http://codereview.chromium.org/8883017/diff/2001/runtime/bin/file.dart#newcode80 runtime/bin/file.dart:80: * input stream must be closed when no ...
9 years ago (2011-12-08 17:25:57 UTC) #2
Mads Ager (google)
9 years ago (2011-12-08 19:37:01 UTC) #3
http://codereview.chromium.org/8883017/diff/2001/runtime/bin/file.dart
File runtime/bin/file.dart (right):

http://codereview.chromium.org/8883017/diff/2001/runtime/bin/file.dart#newcode80
runtime/bin/file.dart:80: * input stream must be closed when no longer used.
On 2011/12/08 17:25:58, Søren Gjesse wrote:
> Maybe add "to free up system resources".

Done.

http://codereview.chromium.org/8883017/diff/2001/runtime/bin/file.dart#newcode86
runtime/bin/file.dart:86: * output stream must be closed when no longer used.
On 2011/12/08 17:25:58, Søren Gjesse wrote:
> Ditto.

Done.

http://codereview.chromium.org/8883017/diff/2001/runtime/bin/file_impl.dart
File runtime/bin/file_impl.dart (right):

http://codereview.chromium.org/8883017/diff/2001/runtime/bin/file_impl.dart#n...
runtime/bin/file_impl.dart:621: var handler = (_openHandler != null) ?
_openHandler : (ignore) => null;
On 2011/12/08 17:25:58, Søren Gjesse wrote:
> Shouldn't the dummy open handler close the file again?

Yes, done.

Powered by Google App Engine
This is Rietveld 408576698