Skip to content

Forget about the iterator type after constructing a JoinAll. #286

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

Closed
wants to merge 2 commits into from

Conversation

dwrensha
Copy link
Contributor

Fixes #285.

@dwrensha
Copy link
Contributor Author

The current type parameter situation is left over from before #255, when JoinAll did in fact keep the iterator around as one of its fields.

@alexcrichton
Copy link
Member

This seems reasonable to me, and it seems like we probably won't be storing the iterator in the JoinAll future any time soon as well, so seems fine moving forward. Could you add a test for this as well?

Unfortunately this is a breaking change so this can't land until 0.2, but I don't mind keeping this around until that time.

@alexcrichton alexcrichton added this to the 0.2 release milestone Dec 11, 2016
@dwrensha
Copy link
Contributor Author

Test added.

@alexcrichton
Copy link
Member

Awesome, thanks! I've tagged this to ensure it makes it into 0.2 and we don't forget about it.

@stuhood
Copy link

stuhood commented Feb 5, 2018

@alexcrichton : It looks like 0.2 might be ramping up. Would it be a good time to tackle this in order to help with #662?

@alexcrichton
Copy link
Member

Certainly! I'd recommend coordinating with @cramertj though!

@cramertj
Copy link
Member

cramertj commented Feb 5, 2018

@stuhood Thanks for your help! Can you send a PR to 0.2 in the next hour or so? If not, I'd recommend waiting until later this afternoon once I've had a chance to reorg 0.2 into multiple crates, otherwise the rebase will be a pain.

@stuhood
Copy link

stuhood commented Feb 5, 2018

@cramertj : Should be able to, as this PR is already green.

@cramertj
Copy link
Member

cramertj commented Feb 5, 2018

@stuhood Great! ty :)

@stuhood stuhood mentioned this pull request Feb 5, 2018
@stuhood
Copy link

stuhood commented Feb 5, 2018

@cramertj, @alexcrichton : Opened #732.

@alexcrichton
Copy link
Member

Landed in #732

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.

4 participants