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.

Sign up for GitHub

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

Jump to bottom

Horovod & py3.8 #2764

Merged
merged 1 commit into from
Jul 30, 2020
Merged

Horovod & py3.8 #2764

merged 1 commit into from
Jul 30, 2020

Conversation

Borda
Copy link
Member

@Borda Borda commented Jul 30, 2020 •
edited by yukw777
Loading

What does this PR do?

resolving Horovod as discussed in #2745
see: #2745 (comment)
cc: @tgaddair

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@Borda Borda added the ci Continuous Integration label Jul 30, 2020
@Borda Borda requested a review from yukw777 July 30, 2020 13:11
@mergify mergify bot requested a review from a team July 30, 2020 13:12
@Borda Borda mentioned this pull request Jul 30, 2020
Merged
@tgaddair
Copy link
Contributor

LGTM! Thanks @Borda!

@Borda Borda added the ready PRs ready to be merged label Jul 30, 2020
@codecov
Copy link

codecov bot commented Jul 30, 2020

Codecov Report

Merging #2764 into master will increase coverage by 0%.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #2764   +/-   ##
======================================
  Coverage      90%     90%           
======================================
  Files          82      82           
  Lines        6826    6810   -16     
======================================
- Hits         6150    6146    -4     
+ Misses        676     664   -12

@mergify mergify bot requested a review from a team July 30, 2020 14:49
@mergify mergify bot requested a review from a team July 30, 2020 17:35
@Borda Borda added the priority: 0 High priority task label Jul 30, 2020
nateraw
nateraw approved these changes Jul 30, 2020
View reviewed changes
Copy link
Contributor

@nateraw nateraw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprisingly simple. LGTM 😄

@Borda Borda merged commit bc833fb into master Jul 30, 2020
@Borda Borda deleted the tests/horovod branch July 30, 2020 21:39
@Borda
Copy link
Member Author

Borda commented Jul 30, 2020

Surprisingly simple. LGTM 😄

yeah, we shall keep PR as simple as possible to the review is quick...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nateraw nateraw nateraw approved these changes

@ananyahjha93 ananyahjha93 ananyahjha93 approved these changes

@awaelchli awaelchli awaelchli approved these changes

@yukw777 yukw777 yukw777 approved these changes

@jeremyjordan jeremyjordan Awaiting requested review from jeremyjordan

@justusschock justusschock Awaiting requested review from justusschock justusschock is a code owner

Assignees
No one assigned
Labels
ci Continuous Integration priority: 0 High priority task ready PRs ready to be merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Borda @tgaddair @yukw777 @awaelchli @ananyahjha93 @nateraw