BF/RF: create-sibling follow diff with constant_refs=False + operate on committed subds #6603
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…on committed Initial motivation for this change was fixing datalad#6596 where while specifying --since, create-sibling did not detect needing to create a sibling for a subdataset. A stale (missed during datalad#6528 OPT/RF) comment said that value of constant_refs=True should not matter since it was not recursive originally. But now here we do recurse and it does matter since we migth completely skip subtree where such refs are not yet known. Changing to False resulted in the test failing which made me realize that sibling was created even for not yet committed changes to submodules. Since for push it would really not work to push something which is not yet saved, I think the change in behavior for create-sibling is also desired. And it fixes datalad#6596. So I have "fixed" and expanded test with the problematic case I had discovered.
This actually led to the tests fails in datalad-deprecated which trigger this tests code path, which is otherwise not used in core
|
ha, due to the ingenuity of the setup, fixes to get datalad-deprecated green are needed in the test code base of the datalad itself. Pushed 5f34a9b . |
|
Code Climate has analyzed commit 5f34a9b and detected 0 issues on this pull request. View more on Code Climate. |
Codecov Report
@@ Coverage Diff @@
## master #6603 +/- ##
==========================================
+ Coverage 91.14% 91.16% +0.01%
==========================================
Files 350 353 +3
Lines 44232 44412 +180
==========================================
+ Hits 40316 40487 +171
- Misses 3916 3925 +9
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Initial motivation for this change was fixing
#6596
where while specifying --since, create-sibling did not detect needing to
create a sibling for a subdataset. A stale (missed during #6528 OPT/RF)
comment said that value of constant_refs=True should not matter since it was
not recursive originally. But now here we do recurse and it does matter since
we migth completely skip subtree where such refs are not yet known. Changing
to False resulted in the test failing which made me realize that sibling was
created even for not yet committed changes to submodules. Since for push it
would really not work to push something which is not yet saved, I think the
change in behavior for create-sibling is also desired. And it fixes #6596.
So I have "fixed" and expanded test with the problematic case I had
discovered.
Changelog
🐛 Bug Fixes
create-siblingwill no longer create siblings for not yet saved new subdatasets, andwill now create sub-datasets nested in the subdatasets which did not yet have those
siblings. Fixes "No datasets qualify for sibling creation." although some should be created #6596