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

Issue 12049013: Change singleSubscription/multiSubscription to normal/broadcast. (Closed)

Created:
7 years, 11 months ago by Lasse Reichstein Nielsen
Modified:
7 years, 11 months ago
Reviewers:
Bob Nystrom, floitsch
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Change singleSubscription/multiSubscription to normal/broadcast. Also make StreamController not a Stream. Committed: https://code.google.com/p/dart/source/detail?r=17563

Patch Set 1 #

Total comments: 8

Patch Set 2 : Addressed comments, renamed .multiSubscription to .broadcast. #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+176 lines, -192 lines) Patch
M sdk/lib/async/stream.dart View 5 chunks +18 lines, -16 lines 1 comment Download
M sdk/lib/async/stream_controller.dart View 1 7 chunks +19 lines, -47 lines 2 comments Download
M sdk/lib/async/stream_impl.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M sdk/lib/async/stream_pipe.dart View 1 chunk +3 lines, -1 line 0 comments Download
M sdk/lib/isolate/isolate_stream.dart View 1 2 chunks +5 lines, -5 lines 0 comments Download
M tests/lib/async/merge_stream_test.dart View 1 7 chunks +21 lines, -21 lines 0 comments Download
M tests/lib/async/stream_controller_async_test.dart View 1 18 chunks +38 lines, -32 lines 0 comments Download
M tests/lib/async/stream_controller_test.dart View 1 17 chunks +52 lines, -50 lines 0 comments Download
M tests/lib/async/stream_min_max_test.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M tests/lib/async/stream_single_test.dart View 3 chunks +5 lines, -5 lines 0 comments Download
M tests/lib/async/stream_single_to_multi_subscriber_test.dart View 1 3 chunks +4 lines, -4 lines 0 comments Download
M utils/pub/error_group.dart View 1 1 chunk +3 lines, -3 lines 0 comments Download
M utils/tests/pub/error_group_test.dart View 1 3 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Lasse Reichstein Nielsen
7 years, 11 months ago (2013-01-22 13:13:24 UTC) #1
floitsch
LGTM. https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controller.dart File sdk/lib/async/stream_controller.dart (left): https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controller.dart#oldcode24 sdk/lib/async/stream_controller.dart:24: class StreamController<T> extends Stream<T> implements StreamSink<T> { This ...
7 years, 11 months ago (2013-01-22 14:37:56 UTC) #2
Lasse Reichstein Nielsen
https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controller.dart File sdk/lib/async/stream_controller.dart (left): https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controller.dart#oldcode24 sdk/lib/async/stream_controller.dart:24: class StreamController<T> extends Stream<T> implements StreamSink<T> { Description updated. ...
7 years, 11 months ago (2013-01-22 15:33:37 UTC) #3
Lasse Reichstein Nielsen
PTAL (and feel free to commit it before I get back, if you want to)
7 years, 11 months ago (2013-01-22 15:35:30 UTC) #4
floitsch
Still LGTM. https://codereview.chromium.org/12049013/diff/5001/sdk/lib/async/stream_controller.dart File sdk/lib/async/stream_controller.dart (right): https://codereview.chromium.org/12049013/diff/5001/sdk/lib/async/stream_controller.dart#newcode12 sdk/lib/async/stream_controller.dart:12: * A controller with the stream it ...
7 years, 11 months ago (2013-01-24 13:03:42 UTC) #5
Bob Nystrom
7 years, 11 months ago (2013-01-24 17:29:14 UTC) #6
Message was sent while issue was closed.
https://codereview.chromium.org/12049013/diff/5001/sdk/lib/async/stream.dart
File sdk/lib/async/stream.dart (right):

https://codereview.chromium.org/12049013/diff/5001/sdk/lib/async/stream.dart#...
sdk/lib/async/stream.dart:83: bool get isBroadcast => false;
Have you considered having an actual distinct type for broadcast streams?

I ask because I know Nathan got burned a couple of times trying to add a
listener to a single-cast stream that already had one. It would be nice if the
type system could help us statically avoid those bugs.

Powered by Google App Engine
This is Rietveld 408576698