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

Fix save to not force committing into git if reference dataset is pure git (not git-annex) #7355

Merged
merged 5 commits into from
Mar 25, 2023

Conversation

yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Mar 24, 2023

Sits on top of #7353 to reuse existing testing setup. Actual fix is few character change in save.py and is present in the last non-changelog commit.

Closes #7351

otherwise there is no point of having two tests - one for
GitRepo and one for AnnexRepo
…aset.create

otherwise it becomes an AnnexRepo and thus behaving pretty much the same way
as in AnnexRepo test and actually test running the same duration. With this change we get

11.45s call     datalad/support/tests/test_repo_save.py::test_annexrepo_save_all
8.14s call     datalad/support/tests/test_repo_save.py::test_gitrepo_save_all

thus attesting to the fact that we get different testing for GitRepo case
Although the file is about .repo.save and not ds.save, there is already a good
number of other tests which pretty much test ds.save.  It is also the only
place which already uses that convoluted scenario so I decided it would be good
place for an addition of another testing thus not introducing much of extra run
time as if I was to create an independent new test for this problem.
@yarikoptic yarikoptic added semver-patch Increment the patch version when merged CHANGELOG-missing When a PR's description does not contain a changelog item, yet. labels Mar 24, 2023
@yarikoptic yarikoptic requested a review from mih March 24, 2023 16:19
@github-actions github-actions bot removed the CHANGELOG-missing When a PR's description does not contain a changelog item, yet. label Mar 24, 2023
@codecov
Copy link

codecov bot commented Mar 24, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.97 🎉

Comparison is base (7557591) 88.73% compared to head (a40592c) 90.70%.

Additional details and impacted files
@@            Coverage Diff             @@
##            maint    #7355      +/-   ##
==========================================
+ Coverage   88.73%   90.70%   +1.97%     
==========================================
  Files         327      327              
  Lines       44567    44569       +2     
  Branches     5906     5906              
==========================================
+ Hits        39545    40428     +883     
+ Misses       5007     4126     -881     
  Partials       15       15              
Impacted Files Coverage Δ
datalad/core/local/save.py 98.91% <ø> (ø)
datalad/support/tests/test_repo_save.py 100.00% <100.00%> (ø)
datalad/tests/utils_pytest.py 89.22% <100.00%> (ø)

... and 4 files with indirect coverage changes

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yarikoptic
Copy link
Member Author

ha -- #7353 lacked codecov report so might be due to that but here we even got 90.40% (+1.66%) boost in coverage.

@yarikoptic
Copy link
Member Author

I would even say that we as well release after this fix since it would already be useful to some folks I know ;-)

@yarikoptic yarikoptic added the release Create a release when this pr is merged label Mar 24, 2023
@yarikoptic
Copy link
Member Author

It is as green as you can ever see a PR here, with even +1.97% boost in coverage -- what could be better? ;)

I will merge unless objections voiced on Sunday.

@yarikoptic yarikoptic added the merge-if-ok OP considers this work done, and requests review/merge label Mar 24, 2023
@yarikoptic yarikoptic merged commit a8d7c63 into datalad:maint Mar 25, 2023
@yarikoptic-gitmate
Copy link
Collaborator

PR released in 0.18.3

@yarikoptic yarikoptic deleted the bf-save branch November 27, 2023 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-if-ok OP considers this work done, and requests review/merge release Create a release when this pr is merged semver-patch Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

non-annex superdataset causes saving in subdataset to use git add not git-annex add
2 participants