Introduce the "getting started with the SWH API" document, as showed on [1]. This is basically a rst export of the document, with minor fixes to remove name/castalia header.
Details
- Reviewers
vlorentz - Group Reviewers
Reviewers - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- rDDOC17951eafe724: Merge branch 'master' of https://forge.softwareheritage.org/source/swh-docs
rDDOC6afe58358b84: T3339 Getting started with the SWH API.
No test
Diff Detail
- Repository
- rDDOC Development documentation
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Thanks.
I did not read everything yet, i'm posting my few comments (right now so i don't forget later) which are mostly nitpicks or typos.
docs/getting-started/getting_started_with_the_swh_api.rst | ||
---|---|---|
18 ↗ | (On Diff #20681) | |
46 ↗ | (On Diff #20681) | |
48 ↗ | (On Diff #20681) | |
49 ↗ | (On Diff #20681) | |
55 ↗ | (On Diff #20681) | |
112 ↗ | (On Diff #20681) | |
118 ↗ | (On Diff #20681) | |
209–213 ↗ | (On Diff #20681) | nitpicks, we tend to:
|
Looks good to me. I have a bunch of pedantic comments (terminology and ReST syntax) though, see below.
IMO the filename should be: docs/getting-started/api.rst to avoid redundancy
And please follow https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#sections for the title hierarchy (start with ======)
docs/getting-started/getting_started_with_the_swh_api.rst | ||
---|---|---|
21–24 ↗ | (On Diff #20681) | Use references instead of external links: https://www.sphinx-doc.org/en/master/usage/restructuredtext/roles.html#ref-role You'll need to edit swh-dataset's documentation to add an anchor there first |
38–58 ↗ | (On Diff #20681) | Isn't this description redundant with the data model? https://docs.softwareheritage.org/devel/swh-model/data-model.html |
72–82 ↗ | (On Diff #20681) | This assumes a virtualenv to work. Could you either mention virtualenvs in the documentation, or update it to work also without a virtualenv (you just need to rename python to python3 and pip to pip3)? |
126–136 ↗ | (On Diff #20681) | what is \*? Shouldn't it be a list? |
330–333 ↗ | (On Diff #20681) | Only to branches and releases. Revisions and content are only linked indirectly |
364 ↗ | (On Diff #20681) | |
427–428 ↗ | (On Diff #20681) | |
475 ↗ | (On Diff #20681) | "associated" implies a mutual relationship |
484–485 ↗ | (On Diff #20681) | they are not the contents themselves, as "name" and "perms" are part of the directory manifest, not the content |
505 ↗ | (On Diff #20681) | ditto |
519–520 ↗ | (On Diff #20681) | |
529 ↗ | (On Diff #20681) | Uncomment / remove? |
556 ↗ | (On Diff #20681) | Use a reference here too (you'll need to add an anchor in swh-vault's documentation) |
564–566 ↗ | (On Diff #20681) | |
579–580 ↗ | (On Diff #20681) | ditto |
- Merge branch 'master' into T3339_getting_started_api
- Fix url to https in sphinx.rst.
- Fix api getting started + rename file.
- More fixes to api getting started.
Thanks!
One last thing:
docs/getting-started/index.rst | ||
---|---|---|
13 | should should be just api, as the file was renamed |
- Merge branch 'master' into T3339_getting_started_api
- Fix wrong index after api rename.
@vlorentz Thanks for the review! Updated diff with your last comment (api in index.rst). I squashed commits and am ready to push to master, as soon as you tell me to.
docs/getting-started/getting_started_with_the_swh_api.rst | ||
---|---|---|
38–58 ↗ | (On Diff #20681) | Well, it's a subset of concepts needed to quickly understand the document, instead of the big, comprehensive but complex, data model. When I'm reading a getting started, I don't want to have to swallow the full mindblowing range of great concepts, I just want to get started, so.. If I may stand up for that, I will, otherwise I'll cut the description short as proposed (and that would be ok ;-). |
72–82 ↗ | (On Diff #20681) | I'd say that "no assumption" is the way to go, so applied what you proposed (no virtualenv). |
126–136 ↗ | (On Diff #20681) | It should indeed! |
209–213 ↗ | (On Diff #20681) | Fixed for the f-string syntax, but I'm not sure about the fix for the black formatting. All code blocks have been switched from ipython3 to python, hope it will fix it. |
- T3339 Getting started with the SWH API.
- Merge branch 'master' of https://forge.softwareheritage.org/source/swh-docs