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 argument-spec for siblings and improve usage synopsis #5913

Merged
merged 1 commit into from Aug 18, 2021

Conversation

mih
Copy link
Member

@mih mih commented Aug 18, 2021

None was declared as supported action, but if given, would cause a crash.

Synopsis went from

Usage: datalad siblings [-h] [-d DATASET] [-s NAME] [--url [URL]] [--pushurl PUSHURL] [-D DESCRIPTION] [--fetch]
                        [--as-common-datasrc NAME] [--publish-depends SIBLINGNAME] [--publish-by-default REFSPEC]
                        [--annex-wanted EXPR] [--annex-required EXPR] [--annex-group EXPR] [--annex-groupwanted EXPR]
                        [--inherit] [--no-annex-info] [-r] [-R LEVELS] [--version]
                        [ACTION]

to

Usage: datalad siblings [-h] [-d DATASET] [-s NAME] [--url [URL]] [--pushurl PUSHURL] [-D DESCRIPTION] [--fetch]
                        [--as-common-datasrc NAME] [--publish-depends SIBLINGNAME] [--publish-by-default REFSPEC]
                        [--annex-wanted EXPR] [--annex-required EXPR] [--annex-group EXPR] [--annex-groupwanted EXPR]
                        [--inherit] [--no-annex-info] [-r] [-R LEVELS] [--version]
                        [{query|add|remove|configure|enable}]

Fixes #5912

`None` was declared as supported `action`, but if given, would cause a
crash.

Synopsis went from

```
Usage: datalad siblings [-h] [-d DATASET] [-s NAME] [--url [URL]] [--pushurl PUSHURL] [-D DESCRIPTION] [--fetch]
                        [--as-common-datasrc NAME] [--publish-depends SIBLINGNAME] [--publish-by-default REFSPEC]
                        [--annex-wanted EXPR] [--annex-required EXPR] [--annex-group EXPR] [--annex-groupwanted EXPR]
                        [--inherit] [--no-annex-info] [-r] [-R LEVELS] [--version]
                        [ACTION]
```

to

```
Usage: datalad siblings [-h] [-d DATASET] [-s NAME] [--url [URL]] [--pushurl PUSHURL] [-D DESCRIPTION] [--fetch]
                        [--as-common-datasrc NAME] [--publish-depends SIBLINGNAME] [--publish-by-default REFSPEC]
                        [--annex-wanted EXPR] [--annex-required EXPR] [--annex-group EXPR] [--annex-groupwanted EXPR]
                        [--inherit] [--no-annex-info] [-r] [-R LEVELS] [--version]
                        [{query|add|remove|configure|enable}]
```

Fixes datalad#5912
@mih mih linked an issue Aug 18, 2021 that may be closed by this pull request
@mih mih added the semver-patch Increment the patch version when merged label Aug 18, 2021
@codecov
Copy link

codecov bot commented Aug 18, 2021

Codecov Report

Merging #5913 (b039c35) into maint (c6e0e4d) will decrease coverage by 60.77%.
The diff coverage is n/a.

❗ Current head b039c35 differs from pull request most recent head 95ac8ad. Consider uploading reports for the commit 95ac8ad to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##            maint    #5913       +/-   ##
===========================================
- Coverage   90.31%   29.54%   -60.78%     
===========================================
  Files         300      297        -3     
  Lines       42402    42366       -36     
===========================================
- Hits        38296    12517    -25779     
- Misses       4106    29849    +25743     
Impacted Files Coverage Δ
datalad/distribution/siblings.py 15.43% <ø> (-65.58%) ⬇️
datalad/tests/test_api.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/support/digests.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/tests/test_base.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/tests/test_config.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/ui/tests/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/tests/test__main__.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/tests/test_strings.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/ui/tests/test_base.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/support/tests/utils.py 0.00% <0.00%> (-100.00%) ⬇️
... and 247 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 c6e0e4d...95ac8ad. Read the comment docs.

@mih
Copy link
Member Author

mih commented Aug 18, 2021

Appveyor tests for mac do not start. But there is nothing platform specific about this change.

@bpoldrack
Copy link
Member

Tried (since I had a vague recollection of EnsureNone being needed when it's not super obvious) and LGTM.

@bpoldrack bpoldrack merged commit d2b8026 into datalad:maint Aug 18, 2021
@mih mih deleted the bf-5912 branch August 18, 2021 14:02
@mih
Copy link
Member Author

mih commented Aug 18, 2021

Thx!

@yarikoptic
Copy link
Member

FWIW that allowing None was there since "inception" of the mode in 2ad0be6 , not added to "fix" something, so this fix should not be stepping on some toes, good.

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.

Inaccessible action choices for siblings
3 participants