-
Notifications
You must be signed in to change notification settings - Fork 111
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
Remove with_testrepos from core package #7176
Conversation
Following the proposal in datalad#6752
Codecov ReportBase: 88.56% // Head: 90.68% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #7176 +/- ##
==========================================
+ Coverage 88.56% 90.68% +2.12%
==========================================
Files 325 324 -1
Lines 44144 43896 -248
Branches 5863 5835 -28
==========================================
+ Hits 39095 39808 +713
+ Misses 5034 4073 -961
Partials 15 15
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good to me.
However, considering the last time moving things to deprecated - could you temporarily commit a change in requirements, so that the two PRs (here and and in depreacted) are testing against each other rather than the released version of the other one, @adswa ?
Just git+http://github.com/datalad/datalad@branch
in requirements-devel in datalad-deprecated and the same way of installing deprecated in the extension test workflow in here.
If those build turn out green, then I'm happy to remove the commit again and merge.
721f3d6
to
0ba9f57
Compare
It took a bit of fiddling, but the two PRs testing against eachother pass. See here for deprecated: datalad/datalad-deprecated#80 I'll cancel the other tests (no need to waste that CO2 and wait for a sign of @bpoldrack to remove the temporary commit, and then I'll also squash the commits in here together. |
Thanks, @adswa! Please go ahead! |
Even more with_testrepos tests Remove more dedicated with_testrepos tests and testrepos-related helper remove testrepos related config / check MNT: Remove with_testrepos decorator and its assertion from testutil test
It is migrated to deprecated
0ba9f57
to
938adf2
Compare
Code Climate has analyzed commit 938adf2 and detected 0 issues on this pull request. View more on Code Climate. |
In which order should this PR and the companion PR in deprecated be merged? |
Ideally deprecated first (+release), since otherwise we'd introduce CI failures with the extension tests here into master, affecting every PR. |
I left a general ping in the PR in deprecated asking for a second pair of eyes/review (wouldn't want to merge or release my own PR without others chiming in there) |
DataLad deprecated now has |
PR released in |
Hopefully fixes #6752 and accompanies datalad/datalad-deprecated#80. It rips out the
with_testrepos
decorator and associated tests for it. I committed in very small chunks in hopes to easier fix what I might accidentally break with this when the CI runs; I plan to squash things together when this turns out to work.🤞
The last commit cleans up a few imports and removes imports that seemed unused.