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

DOC: Document ria url formats in docstring #6861

Merged
merged 6 commits into from
Oct 20, 2022
Merged

DOC: Document ria url formats in docstring #6861

merged 6 commits into from
Oct 20, 2022

Conversation

adswa
Copy link
Member

@adswa adswa commented Jul 20, 2022

@bpoldrack, I made a start at fixing #6800, which asks for docs for RIA URLs. Can you fill in the TODO in this docstring?

This PR can also be a place for additional commits that fix #6860

Changelog

📝 Documentation

  • create-sibling-ria's docstring now defines the schema of RIA URLs and clarifies internal layout of a RIA store.

@adswa adswa added the semver-documentation Changes only affect the documentation, no impact on version label Jul 20, 2022
@mih
Copy link
Member

mih commented Jul 22, 2022

Ping @bpoldrack

@bpoldrack
Copy link
Member

@adswa : Pushed. Is that commit fine with you?

- Complete RIA-URL section and use the 'scheme' term in context of a URL
  instead of 'protocol'
- Add a warning on in-store layout with respect to its git-annex object
  tree
- Point out, how both - bare repo and object tree - are actually
  optional

[ci skip]
UUID or a ~ symbol followed by the dataset's alias name:
'ria+[scheme]://<storelocation>#<dataset-UUID>' or
'ria+[scheme]://<storelocation>#~<aliasname>'.
In addition, specific version identifiers can be appended to the URL with an
Copy link
Member Author

Choose a reason for hiding this comment

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

Citing the original issue:

Adding @something to a working URL seems to look for a branch with that name, seems not to work for other tree-ish (commit, tag).

Does it only work for branches? If if only works for branches, should that be made clear?

Copy link
Member

@bpoldrack bpoldrack Aug 1, 2022

Choose a reason for hiding this comment

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

True. It's passed onto the --branch option to git clone, which considers branches and tags.
We'd probably want to change that, I think, but that's how it is and doc should reflect it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would you mind pushing a clarification?

@adswa
Copy link
Member Author

adswa commented Aug 1, 2022

Thanks! I just re-raised one question from the original issue. And maybe we should also add examples. I can see if I have time today

Co-authored-by: Benjamin Poldrack <bpoldrack@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Aug 1, 2022

Codecov Report

Base: 90.27% // Head: 91.17% // Increases project coverage by +0.90% 🎉

Coverage data is based on head (d1b6a36) compared to base (afc2ecf).
Patch has no changes to coverable lines.

❗ Current head d1b6a36 differs from pull request most recent head 1a7f653. Consider uploading reports for the commit 1a7f653 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6861      +/-   ##
==========================================
+ Coverage   90.27%   91.17%   +0.90%     
==========================================
  Files         354      354              
  Lines       46108    46399     +291     
==========================================
+ Hits        41622    42303     +681     
+ Misses       4486     4096     -390     
Impacted Files Coverage Δ
datalad/distributed/create_sibling_ria.py 81.76% <ø> (ø)
datalad/downloaders/tests/utils.py 92.30% <0.00%> (-7.70%) ⬇️
datalad/api.py 91.11% <0.00%> (-3.63%) ⬇️
datalad/metadata/extractors/image.py 96.77% <0.00%> (-3.23%) ⬇️
datalad/metadata/extractors/exif.py 96.87% <0.00%> (-3.13%) ⬇️
datalad/tests/test_version.py 80.00% <0.00%> (-2.86%) ⬇️
datalad/metadata/extractors/audio.py 97.14% <0.00%> (-2.86%) ⬇️
datalad/support/entrypoints.py 57.89% <0.00%> (-2.64%) ⬇️
datalad/cli/tests/test_formatters.py 97.82% <0.00%> (-2.18%) ⬇️
datalad/local/remove.py 95.38% <0.00%> (-1.54%) ⬇️
... and 25 more

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

@mih mih mentioned this pull request Sep 8, 2022
@mih
Copy link
Member

mih commented Sep 16, 2022

I am putting this into draft mode. There has been no activity in a while. Please close, if abandoned.

@mih mih marked this pull request as draft September 16, 2022 08:17
@adswa
Copy link
Member Author

adswa commented Sep 16, 2022

Oh I think the PR can actually just go in

@adswa
Copy link
Member Author

adswa commented Oct 19, 2022

I have pushed a commit rephrasing the layout section into something that I feel is more pragmatic/non-technical and less vaguely scary. It would be nice to get this vetted by @bpoldrack so that the tiny doc fix for #6800 can finally make it in.

@adswa adswa marked this pull request as ready for review October 19, 2022 20:10
@codeclimate
Copy link

codeclimate bot commented Oct 19, 2022

Code Climate has analyzed commit 325fb51 and detected 0 issues on this pull request.

View more on Code Climate.

@adswa adswa changed the title DOC: Start to document ria url formats in docstring - needs help DOC: Document ria url formats in docstring Oct 19, 2022
@bpoldrack
Copy link
Member

It would be nice to get this vetted by @bpoldrack so that the tiny doc fix for #6800 can finally make it in.

Yes, looks good to me. Thank you, @adswa.

@bpoldrack bpoldrack merged commit 2346326 into master Oct 20, 2022
@bpoldrack bpoldrack deleted the doc-6800 branch October 20, 2022 08:21
@yarikoptic-gitmate
Copy link
Collaborator

PR released in 0.18.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-documentation Changes only affect the documentation, no impact on version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: create-sibling-ria lacks examples
4 participants