Improve documentation of RPC services, methods, messages, fields and events

htdvisser commented 5 years ago

Summary:

We should improve the (generated) API documentation by adding more extensive comments to our protos.

See this PR for a gold standard API comments reference -> https://github.com/TheThingsNetwork/lorawan-stack/pull/3181/files#diff-aa11ad1a94ecc7c1db3ead25d3372bdd39c6a25dd998a2902da6e4c1cb3b8fdbR129

Why do we need this?

This will really improve the developer experience of SDK and application developers.

What is already there? What do you see now?

There's already a lot of documentation on fields, but less on services, methods and messages.

What is missing? What do you want to see?

[x] Improved documentation on messages (for example, explaining what the effects of field masks are in UpdateXXXRequest messages)
[ ] Improved documentation on methods (for example, explaining that As.SetLink does not return whether the actual linking is successful, perhaps explaining how many resources a method call consumes)
[ ] Improved documentation on services (for example, being more clear about which components expose the services, which fields are accepted by the different EndDevice registries).
[x] Improved documentation on events that are published by components (for example, explaining that AS reports as.link.start, as.link.stop and as.link.fail, GS reports similar events, etc)

How do you propose to implement this?

I suppose this can be a long-running issue, and we can gradually add more comments when we feel like it.

What can you do yourself and what do you need help with?

I think it would be best if I went through the Identity Server services, methods and the related messages+fields. @rvolosatovs and @johanstokking would do the same for the different EndDevice registries, and @johanstokking would work on the GS and AS services and methods for linking/link stats.

johanstokking commented 5 years ago

Updated OP and title to include events to the scope of this issue.

htdvisser commented 3 years ago

@benolayinka please see what's missing and assign "experts"/"component owners" to specific tasks.

benolayinka commented 3 years ago

@htdvisser I don't know enough about the RPC stuff to know what is missing. Can this issue be closed, leaving Roman's part open? -> https://github.com/TheThingsNetwork/lorawan-stack/issues/3167

htdvisser commented 3 years ago

This has nothing to do with "knowing enough about the RPC stuff", you need to go through the pages under https://thethingsstack.io/reference/api/ and list the services, methods, messages and fields that need improved documentation. If you can make those improvements yourself, that would be great, and otherwise ask code owners for their expertise.

Example(1): The EntityRegistrySearch.SearchApplications doesn't have a description. This can be added by adding a comment above the RPC:

https://github.com/TheThingsNetwork/lorawan-stack/blob/a738af3a6b65110ebe59948ee4258f6a124e5662/api/search_services.proto#L126

Example(2): It may be good to document in the SetOrganizationCollaboratorRequest message that the collaborator can only be a user, and that you can't make organizations member of other organizations. This is already documented in GetOrganizationCollaboratorRequest, but not in SetOrganizationCollaboratorRequest. It can be added by adding a comment to the message definition:

https://github.com/TheThingsNetwork/lorawan-stack/blob/a738af3a6b65110ebe59948ee4258f6a124e5662/api/organization.proto#L128-L131

benolayinka commented 3 years ago

This has nothing to do with "knowing enough about the RPC stuff", you need to go through the pages under https://thethingsstack.io/reference/api/ and list the services, methods, messages and fields that need improved documentation. If you can make those improvements yourself, that would be great, and otherwise ask code owners for their expertise.

Example(1): The EntityRegistrySearch.SearchApplications doesn't have a description. This can be added by adding a comment above the RPC:

https://github.com/TheThingsNetwork/lorawan-stack/blob/a738af3a6b65110ebe59948ee4258f6a124e5662/api/search_services.proto#L126

Example(2): It may be good to document in the SetOrganizationCollaboratorRequest message that the collaborator can only be a user, and that you can't make organizations member of other organizations. This is already documented in GetOrganizationCollaboratorRequest, but not in SetOrganizationCollaboratorRequest. It can be added by adding a comment to the message definition:

https://github.com/TheThingsNetwork/lorawan-stack/blob/a738af3a6b65110ebe59948ee4258f6a124e5662/api/organization.proto#L128-L131

This is exactly my point - I don't know anything about EntityRegistrySearch.SearchApplications. What I write will be exactly what someone would guess from reading the title, which adds no value.

It will also take me more time to go through and tag people responsible for individual methods than it would for codeowners to scroll through their APIs and add things they know offhand, like This also sets the given organization or user as first collaborator with all possible rights, or As.SetLink does not return whether the actual linking is successful.

I created sub issues for @htdvisser, @johanstokking, and @rvolosatovs. Only @rvolosatovs hasn't closed his issue. I can create a new sub-issue based on codeowner for each API page, but I don't have time to make a list of which messages I think are missing - that's easy for codeowners to see when they look at their respective pages, and doesn't seem like a good time/value tradeoff at all.

johanstokking commented 3 years ago

that's easy for codeowners to see when they look at their respective pages, and doesn't seem like a good time/value tradeoff at all.

Agreed.

Please make the issues actionable (few reviewers) and concrete.

benolayinka commented 3 years ago

Closing this as there are actionable issues for specific APIs

TheThingsNetwork / lorawan-stack

Improve documentation of RPC services, methods, messages, fields and events #252