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

Clarify confusing INFO log message from get() on dataset installation #6871

Merged
merged 1 commit into from
Jul 24, 2022

Conversation

mih
Copy link
Member

@mih mih commented Jul 22, 2022

The confusion arises from a message that announces the installation
of a (sub)dataset, even when the dataset is already present.

This change rewords the message to be more valid in this case.

Fixes #6862

The confusion arises from a message that announces the installation
of a (sub)dataset, even when the dataset is already present.

This change rewords the message to be more valid in this case.

Fixes datalad#6862
@mih mih added the semver-patch Increment the patch version when merged label Jul 22, 2022
Copy link
Member

@bpoldrack bpoldrack left a comment

Choose a reason for hiding this comment

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

Issue remains a bit weird in the toplevel case, since this was ensured already and get would fail before otherwise, but I agree - this wording removes the confusion about the message itself.

@mih
Copy link
Member Author

mih commented Jul 22, 2022

Thx for the review. I think all these log messages are a relic, and should be implemented properly as progress reporting. But I have no interest in working on this.

@codecov
Copy link

codecov bot commented Jul 22, 2022

Codecov Report

Merging #6871 (a178c73) into maint (cefecae) will increase coverage by 0.97%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##            maint    #6871      +/-   ##
==========================================
+ Coverage   90.25%   91.23%   +0.97%     
==========================================
  Files         354      354              
  Lines       46113    46127      +14     
==========================================
+ Hits        41621    42083     +462     
+ Misses       4492     4044     -448     
Impacted Files Coverage Δ
datalad/distribution/get.py 96.76% <ø> (+0.40%) ⬆️
datalad/customremotes/tests/test_datalad.py 95.34% <0.00%> (-2.28%) ⬇️
datalad/distribution/create_sibling.py 69.80% <0.00%> (-0.20%) ⬇️
datalad/cli/common_args.py 100.00% <0.00%> (ø)
datalad/tests/test_dochelpers.py 100.00% <0.00%> (ø)
datalad/support/tests/test_network.py 100.00% <0.00%> (ø)
datalad/tests/utils_pytest.py 89.73% <0.00%> (+0.01%) ⬆️
datalad/utils.py 88.17% <0.00%> (+0.01%) ⬆️
datalad/__init__.py 98.00% <0.00%> (+0.04%) ⬆️
datalad/support/tests/test_locking.py 95.87% <0.00%> (+0.04%) ⬆️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cefecae...a178c73. Read the comment docs.

@yarikoptic yarikoptic merged commit b0500a5 into datalad:maint Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants