osu-web
osu-web copied to clipboard
ppy
Various documentation improvements
The goal of this PR is to make various QOL improvements to the documentation of the osu! v2 API.
- [X] Some multiline comments used for phpdoc documentation are missing asterisks on certain lines (df83a80).
- [X] Some documented api routes have their group set at function declaration, instead of at class declaration (7193011).
- [X] Bump knuckleswtf/scribe to 2.7.10 (27230b0).
- [ ] All of the endpoints with specified responses are using a table, instead of the built-in @responseField. I have an example commit ready of how an endpoint would look like using @responseField instead. I just need a go-ahead to push the commit.
- [ ] Some endpoints no longer match the provided documentation.
- [ ] There are various undocumented endpoints that should be documented.
Out of scope of this PR, but a debate topic:
- Bump major scribe version to v3, or possibly even the newly released v4.
Feedback would be greatly appreciated, especially regarding task 4 and 6, as well as thoughts on migrating to v4.
- Haven't really looked into the history of why responses are documented the way they are, but I assume it is due to the documentation solution that was used before scribe was adopted. Can I get a confirmation whether or not that is true?
- Regarding the endpoints whose types link to object structures, should we provide consistent wording in the field description? Something like
<Type>, <AnotherType> or <ThirdType> type. Note: <Type> includes <parameter>.(e.g.: NewsPost[] type. Note:NewsPostincludespreview). - Should we add proper examples to all of the endpoints, or are they intentionally left out because of clutter on page?
- Are all of the undocumented endpoints finalized to the point, so that they can be documented?
- Scribe v3 had some breaking changes regarding the way osu-web does documentation, I can go into further detail on a separate PR for version bump, but I'd need help with that since I am not that familiar with php myself.
Commit f558bd9 shows off the changes I was talking about.
As for how it looks:
Before:
After:
Limitation with this approach are the types, since only the basic types are supported, but links to object structures can be added to descriptions like it's been done for url, query and body parameters.
Another thing I forgot in the original comment is discussion on how to implement response types where the entire response is a single object structure. Probably the best solution is something like this.
I don't really feel like this is an improvement in legibility. Sure, it's consistent now, but I'd rather have request and response both be displayed as a table instead, which would make it easier to find the values/parameters you're looking for. Just my opinion though, feel free to dismiss :D
Valid point and I do agree that using up more space for the same amount of content is usually not a good thing. However, here are a few counter-points:
- Body parameters support object collapsing, so larger documents with different nesting can very well turn out to be smaller than the table (this is currently not a thing for response parameters (for some reason), and I feel like an issue should be raised with scribe developers). Example of this can be seen on their demo page.
- The latter response is also slightly larger because while I was at it, I also updated the response to match what is currently returned.
- Using @responseField also means that any documentation parsers can properly create interfaces of responses, without having to write extra logic to handle this specific response layout (this was why I opened up the PR in the first place, so that I don't need to write much more extra logic for the parser I made).
The changes should be done in multiple PRs (comment fixing, documented/undocumented group name placement, scribe update (o latest 2.x or higher, and any further updates).
Haven't really looked into the history of why responses are documented the way they are, but I assume it is due to the documentation solution that was used before scribe was adopted. Can I get a confirmation whether or not that is true?
Yes but there are a few things to be noted about response format.
Sometimes response refer to another section altogether (example) or shared between endpoints at the top level (BeatmapScores for example).
And then there are responses which are just array at the top level and thus "Response Fields" doesn't quite make sense.
Another thing is Get Beatmap scores currently returns two different response format based on the request (not currently documented).
Regarding the endpoints whose types link to object structures, should we provide consistent wording in the field description?
Something consistent would be good, yes.
Should we add proper examples to all of the endpoints, or are they intentionally left out because of clutter on page?
They should all have examples. Those which are missing are just because no one added it yet.
Are all of the undocumented endpoints finalized to the point, so that they can be documented?
Depends on the endpoint. Existing public endpoints are probably fine to document. Also check with @notbakaneko as he may have written some chat related documentations.
Scribe v3 had some breaking changes regarding the way osu-web does documentation, I can go into further detail on a separate PR for version bump, but I'd need help with that since I am not that familiar with php myself.
It'll require quite a few changes from what I can tell based on my last attempt few months ago.
The changes should be done in multiple PRs (comment fixing, documented/undocumented group name placement, scribe update (o latest 2.x or higher, and any further updates).
Got it. Should updating documentation of each endpoint (or file) be done as separate PRs also?
Sometimes response refer to another section altogether (example) or shared between endpoints at the top level (BeatmapScores for example).
In this case, we could just link to Build like the Get Changelog Build does no?
And even if that isn't desired, would it not be covered by this:
Another thing I forgot in the original comment is discussion on how to implement response types where the entire response is a single object structure. Probably the best solution is something like this.
And then there are responses which are just array at the top level and thus "Response Fields" doesn't quite make sense.
This example should cover that.
Another thing is Get Beatmap scores currently returns two different response format based on the request (not currently documented).
This one seems like the most troublesome one. It is possible to add different examples to the section on the right, but @responseField-s do not support that afaik.
What are the two different response formats? Are you referring to whether user_score and userScore are included?
If that is what you mean, we could add a note that user_score and userScore get returned if authenticated with identified scope (maybe open an issue with scribe devs to add support for optional attribute for @responseField).
They should all have examples. Those which are missing are just because no one added it yet.
Should we put example responses in separate .json files, because some tend to be pretty big and can clutter the code? If yes, where should we put them?
On a separate note:
Body parameters support object collapsing, so larger documents with different nesting can very well turn out to be smaller than the table (this is currently not a thing for response parameters (for some reason), and I feel like an issue should be raised with scribe developers). Example of this can be seen on their demo page.
This seems to be a feature as of v4.
I have Scribe 4.2 working at https://github.com/cl8n/osu-web/tree/scribe
there have been improvements to doc generation in the newer versions of Scribe, but the frontend requires a lot of workarounds to get it into a similar state as current -- and some features have been reworked into imo worse UX, like the sidebar has this obnoxious scroll delay when opening new sections, and the sidebar doesn't update automatically as you scroll through the document (I added this part back)
my result there is kind of messy code and it doesn't feel like an upgrade as-is, so I'm not sure what to do with it. working on it made me wonder if it's better to just provide our own web frontend for docs...
The patches in here has been mostly applied. Updates to response section should first change how they're rendered.