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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 295913003: Make Stream.where, etc., be documented as inheriting broadcast state. (Closed)

Created:
6 years, 7 months ago by Lasse Reichstein Nielsen
Modified:
6 years, 7 months ago
CC:
reviews_dartlang.org, floitsch, nweiz
Visibility:
Public.

Description

Make Stream.where, etc., be documented as inheriting broadcast state. Very few changes were necessary because it was already the behavior in most cases. BUG= http://dartbug.com/18586 R=ajohnsen@google.com Committed: https://code.google.com/p/dart/source/detail?r=36344 Committed: https://code.google.com/p/dart/source/detail?r=36475

Patch Set 1 #

Total comments: 2

Patch Set 2 : Do timeout properly, instead of falling back on asBroadcastStream. #

Patch Set 3 : Reapply #

Patch Set 4 : Change asBroadcastStream to always only listen once to its source. #

Total comments: 3

Patch Set 5 : Make asyncMap and asyncExpand also inherit broadcastness. #

Total comments: 2

Patch Set 6 : Also test broadcast.asBroadcast case. #

Patch Set 7 : Add issue number for co19 status changes. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+335 lines, -117 lines) Patch
M sdk/lib/async/stream.dart View 1 2 3 4 17 chunks +144 lines, -115 lines 0 comments Download
M sdk/lib/async/stream_transformers.dart View 1 chunk +2 lines, -0 lines 0 comments Download
M tests/co19/co19-co19.status View 1 2 3 4 5 6 1 chunk +3 lines, -0 lines 0 comments Download
M tests/lib/async/stream_timeout_test.dart View 2 chunks +2 lines, -2 lines 0 comments Download
A tests/lib/async/stream_transformation_broadcast_test.dart View 1 2 3 4 5 1 chunk +184 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
Lasse Reichstein Nielsen
6 years, 7 months ago (2014-05-20 06:57:46 UTC) #1
Anders Johnsen
lgtm https://codereview.chromium.org/295913003/diff/1/sdk/lib/async/stream.dart File sdk/lib/async/stream.dart (right): https://codereview.chromium.org/295913003/diff/1/sdk/lib/async/stream.dart#newcode1167 sdk/lib/async/stream.dart:1167: result = result.asBroadcastStream(); Isn't this problematic? Would it ...
6 years, 7 months ago (2014-05-20 07:11:23 UTC) #2
Lasse Reichstein Nielsen
https://codereview.chromium.org/295913003/diff/1/sdk/lib/async/stream.dart File sdk/lib/async/stream.dart (right): https://codereview.chromium.org/295913003/diff/1/sdk/lib/async/stream.dart#newcode1167 sdk/lib/async/stream.dart:1167: result = result.asBroadcastStream(); On 2014/05/20 07:11:23, Anders Johnsen wrote: ...
6 years, 7 months ago (2014-05-20 07:20:30 UTC) #3
Lasse Reichstein Nielsen
Committed patchset #2 manually as r36344 (presubmit successful).
6 years, 7 months ago (2014-05-20 07:28:20 UTC) #4
Lasse Reichstein Nielsen
PTAL (again). This seems to fix the pkg/analysis_server problem.
6 years, 7 months ago (2014-05-20 11:42:53 UTC) #5
Anders Johnsen
lgtm, I think this is acceptable. https://codereview.chromium.org/295913003/diff/60001/tests/co19/co19-co19.status File tests/co19/co19-co19.status (right): https://codereview.chromium.org/295913003/diff/60001/tests/co19/co19-co19.status#newcode75 tests/co19/co19-co19.status:75: LibTest/async/Stream/asBroadcastStream_A02_t01: Fail File ...
6 years, 7 months ago (2014-05-20 12:01:32 UTC) #6
nweiz
https://codereview.chromium.org/295913003/diff/60001/sdk/lib/async/stream.dart File sdk/lib/async/stream.dart (right): https://codereview.chromium.org/295913003/diff/60001/sdk/lib/async/stream.dart#newcode288 sdk/lib/async/stream.dart:288: * The returned stream is not a broadcast stream. ...
6 years, 7 months ago (2014-05-20 21:15:53 UTC) #7
Lasse Reichstein Nielsen
https://codereview.chromium.org/295913003/diff/60001/sdk/lib/async/stream.dart File sdk/lib/async/stream.dart (right): https://codereview.chromium.org/295913003/diff/60001/sdk/lib/async/stream.dart#newcode288 sdk/lib/async/stream.dart:288: * The returned stream is not a broadcast stream. ...
6 years, 7 months ago (2014-05-21 06:16:52 UTC) #8
Lasse Reichstein Nielsen
Pah, let's just do the proper thing and let asyncMap and asyncExpand return broadcast streams ...
6 years, 7 months ago (2014-05-21 11:58:41 UTC) #9
Anders Johnsen
lgtm https://codereview.chromium.org/295913003/diff/80001/tests/lib/async/stream_transformation_broadcast_test.dart File tests/lib/async/stream_transformation_broadcast_test.dart (right): https://codereview.chromium.org/295913003/diff/80001/tests/lib/async/stream_transformation_broadcast_test.dart#newcode181 tests/lib/async/stream_transformation_broadcast_test.dart:181: (c) => c.stream.asBroadcastStream()); Add new StreamController.broadcast().stream.asBroadcastStream()
6 years, 7 months ago (2014-05-21 12:19:04 UTC) #10
Lasse Reichstein Nielsen
https://codereview.chromium.org/295913003/diff/80001/tests/lib/async/stream_transformation_broadcast_test.dart File tests/lib/async/stream_transformation_broadcast_test.dart (right): https://codereview.chromium.org/295913003/diff/80001/tests/lib/async/stream_transformation_broadcast_test.dart#newcode181 tests/lib/async/stream_transformation_broadcast_test.dart:181: (c) => c.stream.asBroadcastStream()); On 2014/05/21 12:19:05, Anders Johnsen wrote: ...
6 years, 7 months ago (2014-05-21 12:28:23 UTC) #11
Lasse Reichstein Nielsen
6 years, 7 months ago (2014-05-22 08:44:30 UTC) #12
Message was sent while issue was closed. 
Committed patchset #7 manually as r36475 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698