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

Fix async iterator flatmap not flatmapping async iterables #58

Closed
wants to merge 3 commits into from
Closed

Fix async iterator flatmap not flatmapping async iterables #58

wants to merge 3 commits into from

Conversation

Jamesernator
Copy link

This replaces #57 by introducing a new operation GetIteratorOrThrow and replaces the existing GetIterator with one that does no throw if the value is not iterable.

James Browning added 3 commits October 23, 2019 16:23
@bakkot

This comment has been minimized.

@zloirock

This comment has been minimized.

@bakkot

This comment has been minimized.

@zloirock
Copy link
Contributor

@Jamesernator could you also add a fix from this decision?

@zloirock

This comment has been minimized.

@devsnek
Copy link
Member

devsnek commented Oct 23, 2019

i am planning to leave this open until 55 is resolved one way or another. until then, its probably best to keep the discussion to 55.

@devsnek devsnek added this to the Stage 3 milestone Oct 23, 2019
@zloirock
Copy link
Contributor

@devsnek hey, without this fix, flatMap is completely broken and we need proposed abstract operations anyway. Maybe better to merge it for making this method in the spec to work at least somehow and after resolving of #55 change it if it will be required?

@devsnek
Copy link
Member

devsnek commented Oct 23, 2019

Async methods are also broken because they don't handle errors correctly yet. I'd like to worry about our intention first, and then we can worry about how to spec our intention.

@zloirock
Copy link
Contributor

zloirock commented Oct 23, 2019

Yep, but it's 2 different spec issues -) The code from this PR could be reused after any decision on #55.

@bakkot
Copy link
Collaborator

bakkot commented Nov 7, 2019

I believe this is fixed as of #59.

@devsnek
Copy link
Member

devsnek commented Nov 7, 2019

yep.

@devsnek devsnek closed this Nov 7, 2019
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.

None yet

4 participants