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

Symbol.iterator for Result #13

Closed
wants to merge 2 commits into from
Closed

Symbol.iterator for Result #13

wants to merge 2 commits into from

Conversation

jozanza
Copy link

@jozanza jozanza commented Nov 8, 2017

Added the Symbol.iterator method to instances of Result. If everyone's into it, I can write a couple test cases to keep coverage at 100%. Once merged, this would close #12.

It'd be very convenient to be able to pull values out of Results via array destructuring.
@jozanza jozanza changed the title Result Symbol.iterator Symbol.iterator for Result Nov 8, 2017
@jozanza
Copy link
Author

jozanza commented Nov 8, 2017

Looks like the webpack build failed. Does the UglifyJs plugin have something against generator function syntax? 🤔

@SilentCicero
Copy link
Member

SilentCicero commented Nov 8, 2017 via email

@jozanza
Copy link
Author

jozanza commented Nov 8, 2017

Enjoy your break!

@SilentCicero
Copy link
Member

@jozanza can you look into another dev environment that would support your changes? Maybe switching out uglify JS, or looking into that more closely.

Added an plain object iterator to avoid minifier issues
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.7%) to 98.27% when pulling 1056ed5 on jozanza:master into e63f929 on ethjs:master.

@jozanza
Copy link
Author

jozanza commented Nov 20, 2017

@SilentCicero hey sorry for the delay. I updated the PR to use a non-generator iterator, so we shouldn't have to mess with the environment. Now, I just need to add a test to get coverage back up.

@kumavis
Copy link
Contributor

kumavis commented Jun 1, 2018

@jozanza highly desired feature - seems like it needs some test coverage

@jozanza jozanza closed this Jan 18, 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.

Make Results Iterable
4 participants