Page MenuHomeSoftware Heritage

getting_started_api initialise.
ClosedPublic

Authored by borisbaldassari on May 26 2021, 11:54 AM.

Details

Summary

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.

[1] https://deepnote.com/project/Software-Heritage-aHgvv66LTAC1jgEYfe9elw/%2Fgetting_started_with_the_swh_api.ipynb

Test Plan

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:

  • use black to format our base code, might as well show it the same way in samples.
  • (for some time now) use the f-string syntax since it's allowed by the python version we develop with (3.7 onwards).
vlorentz added a subscriber: vlorentz.

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

This revision now requires changes to proceed.May 28 2021, 4:24 PM
borisbaldassari marked 22 inline comments as done.
  • 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

This revision is now accepted and ready to land.Mon, Jul 19, 3:59 PM
  • 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.