Skip to content

Fix 'foreach' comment in AsyncSubscriber and rename SyncSybscriber.foreach to whenNext #289

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

Merged
merged 1 commit into from
Sep 8, 2015

Conversation

rstoyanchev
Copy link
Contributor

AsyncSubscriber had a comment referring to foreach which is a method that exists in SyncSubscriber while in AsyncSubscriber it is called whenNext.

This PR also renames foreach in SyncSubscriber to whenNext. When I first looked at SyncSubscriber I had the impression there was a loop. It doesn't make sense I know since I did look at foreach. Nevertheless it was only when I debugged that I realized that just above is if (!done) and not while (!done).

Feel free to ignore this if you had some strong reason for the naming.

@viktorklang
Copy link
Contributor

@rstoyanchev Thanks for the correction/touch-up! Would you mind adding yourself to the copyright-waivers in this commit and push again so that it can be merged? Thanks, √

@rstoyanchev
Copy link
Contributor Author

Sure thing, done.

@ktoso
Copy link
Contributor

ktoso commented Aug 30, 2015

LGTM from my point of view as well, thanks :)

@viktorklang
Copy link
Contributor

👍

@viktorklang
Copy link
Contributor

Thank you @rstoyanchev!

viktorklang added a commit that referenced this pull request Sep 8, 2015
Fix 'foreach' comment in AsyncSubscriber and rename SyncSybscriber.foreach to whenNext
@viktorklang viktorklang merged commit 61f9963 into reactive-streams:master Sep 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants