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

Clean up test.run code. #146

Merged
merged 3 commits into from
Jul 1, 2019
Merged

Conversation

rillian
Copy link
Contributor

@rillian rillian commented Jun 4, 2019

This is some cleanup of the code around the parallel iterators in Test.run I did while investigating #145.

The file lists returned by the finder are already iterable,
so there's no need to create a new list from them. If there
were just wrapping them in list() is another possiblity.

Also update the comments so they match the code better.

The file lists returned by the finder are already iterable,
so there's no need to create a new list from them. If there
were just wrapping them in `list()` is another possiblity.

Also update the comments so they match the code better.
@coveralls
Copy link

coveralls commented Jun 4, 2019

Coverage Status

Coverage increased (+0.09%) to 95.71% when pulling 93a3238 on rillian:run-cleanup into 0124001 on Capitains:master.

@PonteIneptique
Copy link
Member

PonteIneptique commented Jun 17, 2019

Dear @rillian , my complete apologies. I am in a deep tunnel regarding this. Maybe @sonofmun can chime in ?

@sonofmun
Copy link
Contributor

I should have time to take a look at it later this week. Ping me again if you haven't heard anything by Thursday (June 20).

@rillian
Copy link
Contributor Author

rillian commented Jun 17, 2019

Ok. Thanks for the update!

@PonteIneptique
Copy link
Member

Love the change in comments, definitely a good pr. Thanks !

@PonteIneptique PonteIneptique merged commit 517cae2 into Capitains:master Jul 1, 2019
@rillian rillian deleted the run-cleanup branch July 1, 2019 16:41
@rillian
Copy link
Contributor Author

rillian commented Jul 1, 2019

\o/

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