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

Mocking Loggers (part 3b, comet) #3853

Merged
merged 3 commits into from
Oct 6, 2020
Merged

Mocking Loggers (part 3b, comet) #3853

merged 3 commits into from
Oct 6, 2020

Conversation

awaelchli
Copy link
Contributor

@awaelchli awaelchli commented Oct 5, 2020 •
edited
Loading

Comes after #3851

Part 2 of comet mocking, step by step
this is needed to consistently mock the logger

split in smaller parts in order to keep PRs managable and easy to review.

@awaelchli awaelchli added ci Continuous Integration refactor labels Oct 5, 2020
@mergify mergify bot requested a review from a team October 5, 2020 00:55
@awaelchli awaelchli mentioned this pull request Oct 5, 2020
Merged
@mergify
Copy link
Contributor

mergify bot commented Oct 5, 2020

This pull request is now in conflict... :(

@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #3853 into master will decrease coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #3853   +/-   ##
======================================
- Coverage      88%     88%   -0%     
======================================
  Files         117     117           
  Lines        9132    9129    -3     
======================================
- Hits         7992    7989    -3     
  Misses       1140    1140

justusschock
justusschock approved these changes Oct 5, 2020
View reviewed changes
Copy link
Member

@justusschock justusschock left a comment

Choose a reason for hiding this comment

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

LGTM. Do you know why tests are failing?

@mergify mergify bot requested a review from a team October 5, 2020 07:36
@awaelchli
Copy link
Contributor Author

It is a early stop / checkpointing test failing, but comes from master. Have not investigated yet.

@mergify mergify bot requested a review from a team October 5, 2020 12:33
@awaelchli awaelchli added logger Related to the Loggers and removed refactor labels Oct 5, 2020
@williamFalcon williamFalcon merged commit 893bed7 into master Oct 6, 2020
@awaelchli awaelchli deleted the tests/mock-comet-2 branch October 7, 2020 02:56
@Borda Borda added this to the 0.10.0 milestone Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SkafteNicki SkafteNicki SkafteNicki approved these changes

@justusschock justusschock justusschock approved these changes

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

@williamFalcon williamFalcon Awaiting requested review from williamFalcon

Assignees
No one assigned
Labels
ci Continuous Integration logger Related to the Loggers
Projects
None yet
Milestone
pl:0.10
Development

Successfully merging this pull request may close these issues.
5 participants
@awaelchli @justusschock @SkafteNicki @williamFalcon @Borda