-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
doc: Updates related to ServiceMsg, sdk.Msg and Msg service #9294
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! A lot of minor suggestions and fixes.
Co-authored-by: Ryan Christoffersen <12519942+ryanchrypto@users.noreply.github.com>
Thank you @ryanchrypto for looking at the PRs and improving it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall, proposing to rename "service RPC" to "service method"
docs/basics/app-anatomy.md
Outdated
@@ -139,7 +139,9 @@ Modules must implement [interfaces](../building-modules/module-manager.md#applic | |||
|
|||
### `Msg` Services | |||
|
|||
Each module defines two [Protobuf services](https://developers.google.com/protocol-buffers/docs/proto#services): one `Msg` service to handle messages, and one gRPC `Query` service to handle queries. If we consider the module as a state-machine, then a `Msg` is a state transition. A `Msg` service is a Protobuf service defining all possible `Msg`s a module exposes. Note that `Msg`s are bundled in [`transactions`](../core/transactions.md), and each transaction contains one or multiple `messages`. | |||
Each module defines two [Protobuf services](https://developers.google.com/protocol-buffers/docs/proto#services): one `Msg` service to handle messages, and one gRPC `Query` service to handle queries. If we consider the module as a state-machine, then a `Msg` service is a set of state transition RPC methods. | |||
Each Protobuf `Msg` service RPC is 1:1 related to a Protobuf request type, which must implement `sdk.Msg` interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in the protobuf docs, a service == a RPC service, and that's where RPC is mentioned. What you call "service RPC" here is actually what protobuf calls a method.
My proposal would be to s/service RPC/service method/g
(in all docs), what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about it, I thought that we use method
because of Go, and in proto each endpoint is marked as RPC. But if in prodobuf docs they use method then let's use it here as well. I'm updating the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
Description
In this PR I'm trying to distinguish sdk.Msg from Msg service as well as remove leftovers of ServiceMsg.
There are many places in documentation which were not very clear - I hope this will improve them.
closes: #9208
related to: baseapp audit
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes