Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tutorial(streams): update for Dart 2 and analayze/test code #813

Merged
merged 3 commits into from
Apr 27, 2018

Conversation

chalin
Copy link
Contributor

@chalin chalin commented Apr 23, 2018

@chalin chalin added fix.examples Adds or changes example test.general Relates to unit, integration, perf testing labels Apr 23, 2018
@chalin chalin added this to the 2-beta3 milestone Apr 23, 2018
@chalin chalin requested a review from kwalrath April 23, 2018 20:28
@googlebot googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Apr 23, 2018
@chalin chalin force-pushed the chalin-tutorial-streams-0423 branch from 998a37a to 4301a9a Compare April 27, 2018 15:33
Copy link
Contributor

@kwalrath kwalrath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few quibbles, but I like your changes!


<?code-excerpt "misc/lib/tutorial/stream_interface.dart (main-stream-members)" remove="/^\s*Stream/"?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for the change in order for this list of methods? (The previous order seemed intuitive to me.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see... Getters in alpha order, then methods in alpha order. I guess that's easier to maintain.

{% endprettify %}

The `distinct()` function doesn't exist on Iterable, but it could have.
The `distinct()` function doesn't exist on `Iterable`, but it could have.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch these two sentences to match the new order.

{% endprettify %}

These first seven all correspond to similar methods on Iterable
These correspond to similar methods on [Iterable][]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These -> The preceding methods

A transformer has only one function, `bind()`, which can be
easily implemented by an **async** function.
A [StreamTransformer][] can work with that.
For example, decoders like [utf8.decode()] are transformers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't utf8.decode() be Utf8Decoder (https://api.dartlang.org/dev/2.0.0-dev.50.0/dart-convert/Utf8Decoder-class.html)? We're talking about the decoders/transformers here, not functions.

easily implemented by an **async** function.
A [StreamTransformer][] can work with that.
For example, decoders like [utf8.decode()] are transformers.
A transformer has only one function, [bind()][], which can be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has -> requires

(I was trying not to make any fixes to stuff you didn't change, but it's just not true that every transformer has only one function!)

Stream<S> mapLogErrors<S, T>(
Stream<T> stream,
S Function(T event) convert,
) async* {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice usage of async*!

@chalin chalin merged commit 3dfc8db into master Apr 27, 2018
@chalin chalin deleted the chalin-tutorial-streams-0423 branch April 27, 2018 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Contributor has signed the Contributor License Agreement fix.examples Adds or changes example test.general Relates to unit, integration, perf testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants