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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

prune ignore #4240

Merged
merged 2 commits into from Oct 20, 2020
Merged

prune ignore #4240

merged 2 commits into from Oct 20, 2020

Conversation

Borda
Copy link
Member

@Borda Borda commented Oct 19, 2020

What does this PR do?

prune ignore file in base testing as we do not use sklearn metrics anymore...

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 Oct 19, 2020
@mergify mergify bot requested a review from a team October 19, 2020 18:41
@codecov
Copy link

codecov bot commented Oct 19, 2020 •
edited

Codecov Report

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

@@           Coverage Diff           @@
##           master   #4240    +/-   ##
=======================================
  Coverage      90%     90%            
=======================================
  Files         103     103            
  Lines        7863    8229   +366     
=======================================
+ Hits         7064    7394   +330     
- Misses        799     835    +36

awaelchli
awaelchli approved these changes Oct 19, 2020
View reviewed changes
Copy link
Member

@awaelchli awaelchli left a comment

Choose a reason for hiding this comment

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

you can probably also get rid of the logger ignores.

@mergify mergify bot requested a review from a team October 19, 2020 22:02
@Borda
Copy link
Member Author

Borda commented Oct 19, 2020

you can probably also get rid of the logger ignores.

we still have loggers, right?

@awaelchli
Copy link
Member

awaelchli commented Oct 19, 2020 •
edited

Yes, but in testing we have them mocked, so it will work without loggers installed (ci does not install third party loggers anymore). Maybe just try and revert if it fails :)

@edenlightning edenlightning added this to the 1.0.3 milestone Oct 19, 2020
@edenlightning edenlightning added the ready PRs ready to be merged label Oct 19, 2020
@Borda
Copy link
Member Author

Borda commented Oct 20, 2020

Yes, but in testing we have them mocked, so it will work without loggers installed (ci does not install third party loggers anymore). Maybe just try and revert if it fails :)

ok, lets try...

@mergify mergify bot requested a review from a team October 20, 2020 10:32
@SeanNaren SeanNaren merged commit 9edef40 into master Oct 20, 2020
@ydcjeff ydcjeff deleted the ci/prune-ignore branch October 20, 2020 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@awaelchli awaelchli awaelchli approved these changes

@SeanNaren SeanNaren SeanNaren approved these changes

@ydcjeff ydcjeff ydcjeff approved these changes

@edenlightning edenlightning Awaiting requested review from edenlightning

@lezwon lezwon Awaiting requested review from lezwon

@tchaton tchaton Awaiting requested review from tchaton

Assignees
No one assigned
Labels
ci Continuous Integration ready PRs ready to be merged
Projects
None yet
Milestone
pl:1.0.x
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Borda @awaelchli @SeanNaren @ydcjeff @edenlightning