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

Add missing api docs #1351

Merged
merged 8 commits into from Apr 21, 2021
Merged

Add missing api docs #1351

merged 8 commits into from Apr 21, 2021

Conversation

amCap1712
Copy link
Member

Some of the api endpoints are absent from the api docs at listenbrainz.readthedocs.io. This is because we need to manually add new api blueprints to api.rst. This is only required when we use a new blueprint. Adding a new endpoint to a blueprint that is already documented (present in api.rst) does not need any extra changes.

We have also decided to use code-block directives for specifying json blocks in the documentation.

I think these details are generally useful in coding/reviewing apis and will add these to the checklist (to be created in the wiki).

@pep8speaks
Copy link

pep8speaks commented Mar 18, 2021

Hello @amCap1712! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 64:131: E501 line too long (143 > 130 characters)

Comment last updated at 2021-04-21 14:48:39 UTC

Copy link
Collaborator

@alastair alastair 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 cleaning this up. I have a few questions about some of the new endpoints. It's not clear if we need to document these for users, or if they should stay internal.

@@ -66,6 +69,46 @@ The playlists API allows for the creation and editing of lists of recordings
:include-empty-docstring:
:undoc-static:

Feedback API Endpoints
^^^^^^^^^^^^^^^^^^^^^^
These API endpoints allow to submit and retrieve feedback for a user's recordings
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
These API endpoints allow to submit and retrieve feedback for a user's recordings
These API endpoints allow to submit and retrieve feedback recordings that a user has listened to

docs/dev/api.rst Show resolved Hide resolved
docs/dev/api.rst Outdated
Missing MusicBrainz Data API Endpoints
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
These api endpoints allow to retrieve data which the user has submitted to
ListenBrainz but is missing from MusicBrainz.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand what this API does. Is it things that are missing from MusicBrainz, or things that we've not successfully managed to link to MusicBrainz? What is the way that this is added, and why would a user want to access this data?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is generated during matching the listens with the MBID mapping. The listens could not be matched that are used to create this data. The intent to add this data was for displaying it in some form on LB website so as to nudge users to add this data to MB.

Speaking in general regardless of whether the API is useful to users or not, I think putting it on the docs adds a little to the convenience of developers. I say this because docs are rendered in a more human readable form on readthedocs than the source.

In case, these endpoints adds noise for users, we should probably consider adding another different section to docs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, I'd consider adding this to a separate section not directly on the core submit/get documentation.

Copy link
Collaborator

@alastair alastair left a comment

Choose a reason for hiding this comment

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

other than the comment about splitting the "Missing MusicBrainz Data" section to another page, this is fine

This endpoint is mostly internal for now, and doesn't currently have any
data
@alastair alastair merged commit bd9f670 into master Apr 21, 2021
@alastair alastair deleted the add-missing-docs branch April 21, 2021 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants