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

KBI0028: Notes on Sciebo/Nextcloud share URLs #104

Merged
merged 7 commits into from
Jul 25, 2023
Merged

KBI0028: Notes on Sciebo/Nextcloud share URLs #104

merged 7 commits into from
Jul 25, 2023

Conversation

mslw
Copy link
Contributor

@mslw mslw commented Jul 19, 2023

It did not come up as a support event (i.e. "nobody asked for this"), but this goes back to something we considered for an archival request (internal issue tracker) and something I later explored for myself, so I think knowledge base is an useful place to document that with better visibility.

Features Nextcloud / Sciebo specific details, and a practical example of an uncurl remote. Writing it up was very educational for me.

@mslw mslw marked this pull request as ready for review July 19, 2023 15:16
Copy link
Contributor

@jsheunis jsheunis left a comment

Choose a reason for hiding this comment

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

Thanks for writing this up. Great technical details that are sensible and can be followed easily.

I made some suggestions that are mostly about improving the structure and readability of the document as a whole.

kbi/0028/index.rst Outdated Show resolved Hide resolved
kbi/0028/index.rst Outdated Show resolved Hide resolved
kbi/0028/index.rst Outdated Show resolved Hide resolved
kbi/0028/index.rst Outdated Show resolved Hide resolved
kbi/0028/index.rst Outdated Show resolved Hide resolved
kbi/0028/index.rst Outdated Show resolved Hide resolved
kbi/0028/index.rst Outdated Show resolved Hide resolved
kbi/0028/index.rst Outdated Show resolved Hide resolved
kbi/0028/index.rst Outdated Show resolved Hide resolved
kbi/0028/index.rst Outdated Show resolved Hide resolved
@jsheunis
Copy link
Contributor

This KBI made me think that something like a webdav-import-files command, similar to datalad/datalad-dataverse#292, would be useful functionality.

Copy link
Contributor

@mih mih left a comment

Choose a reason for hiding this comment

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

This is a great addition!

@adswa
Copy link
Contributor

adswa commented Jul 20, 2023

super insightful! Thanks for writing this up!

mslw and others added 2 commits July 20, 2023 17:10
Co-authored-by: Stephan Heunis <s.heunis@fz-juelich.de>
@mslw
Copy link
Contributor Author

mslw commented Jul 20, 2023

Thanks for the reviews!

@jsheunis I applied most of your suggestions, but then pushed further tweaks, sometimes refining the suggestions further. I rejected the suggestion in introduction, but pushed a change that's similar in spirit while remaining a short paragraph (note that :ref: renders the entire title of referenced section).

@mslw mslw requested a review from jsheunis July 20, 2023 15:36
Copy link
Contributor

@jsheunis jsheunis left a comment

Choose a reason for hiding this comment

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

Happy with the edited/committed changes 👍 👍

mslw added 2 commits July 21, 2023 17:50
Discovered that by accident today - registered URL would be urlquoted
anyway, plus uncurl tries to split the match pattern into match
patterns.
Copy link
Contributor

@loj loj left a comment

Choose a reason for hiding this comment

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

This is great to have! Thanks @mslw!

@mslw
Copy link
Contributor Author

mslw commented Jul 25, 2023

Thanks again for the reviews, merging!

@mslw mslw merged commit c28efaf into main Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants