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.

Sign up for GitHub

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

Jump to bottom

chore: Add "Since:" on proto doc comments #10434

Merged
merged 7 commits into from Oct 27, 2021
Merged

chore: Add "Since:" on proto doc comments #10434

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cb3ea4a
chore: Add `@since` on proto doc comments
amaury1093 Oct 25, 2021
be44209
make proto gen
amaury1093 Oct 25, 2021
390a416
Remove v
amaury1093 Oct 25, 2021
1c8a923
Use `Since: cosmos-sdk 0.43`
amaury1093 Oct 26, 2021
f74fd81
Add everywhere else and make-proto-gen
amaury1093 Oct 26, 2021
42d543e
Merge branch 'master' into am/protobuf-since
amaury1093 Oct 26, 2021
83a30b8
Merge branch 'master' into am/protobuf-since
amaury1093 Oct 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 9 additions & 8 deletions client/grpc/tmservice/query.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 18 additions & 4 deletions docs/core/proto-docs.md
View file
Open in desktop
Expand Up @@ -855,7 +855,9 @@ pagination. Ex:
| `offset` | [uint64](#uint64) | | offset is a numeric offset that can be used when key is unavailable. It is less efficient than using key. Only one of offset or key should be set. |
| `limit` | [uint64](#uint64) | | limit is the total number of results to be returned in the result page. If left empty it will default to a value to be set by each app. |
| `count_total` | [bool](#bool) | | count_total is set to true to indicate that the result set should include a count of the total number of items available for pagination in UIs. count_total is only respected when offset is used. It is ignored when key is set. |
| `reverse` | [bool](#bool) | | reverse is set to true if results are to be returned in the descending order. |
| `reverse` | [bool](#bool) | | reverse is set to true if results are to be returned in the descending order.

@since Cosmos SDK 0.43 |



Expand Down Expand Up @@ -1020,6 +1022,8 @@ QueryAccountResponse is the response type for the Query/Account RPC method.
### QueryAccountsRequest
QueryAccountsRequest is the request type for the Query/Accounts RPC method.

@since Cosmos SDK 0.43


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
Expand All @@ -1035,6 +1039,8 @@ QueryAccountsRequest is the request type for the Query/Accounts RPC method.
### QueryAccountsResponse
QueryAccountsResponse is the response type for the Query/Accounts RPC method.

@since Cosmos SDK 0.43


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
Expand Down Expand Up @@ -1109,7 +1115,9 @@ Query defines the gRPC querier service.

| Method Name | Request Type | Response Type | Description | HTTP Verb | Endpoint |
| ----------- | ------------ | ------------- | ------------| ------- | -------- |
| `Accounts` | [QueryAccountsRequest](#cosmos.auth.v1beta1.QueryAccountsRequest) | [QueryAccountsResponse](#cosmos.auth.v1beta1.QueryAccountsResponse) | Accounts returns all the existing accounts | GET|/cosmos/auth/v1beta1/accounts|
| `Accounts` | [QueryAccountsRequest](#cosmos.auth.v1beta1.QueryAccountsRequest) | [QueryAccountsResponse](#cosmos.auth.v1beta1.QueryAccountsResponse) | Accounts returns all the existing accounts

@since Cosmos SDK 0.43 | GET|/cosmos/auth/v1beta1/accounts|
| `Account` | [QueryAccountRequest](#cosmos.auth.v1beta1.QueryAccountRequest) | [QueryAccountResponse](#cosmos.auth.v1beta1.QueryAccountResponse) | Account returns account details based on address. | GET|/cosmos/auth/v1beta1/accounts/{address}|
| `Params` | [QueryParamsRequest](#cosmos.auth.v1beta1.QueryParamsRequest) | [QueryParamsResponse](#cosmos.auth.v1beta1.QueryParamsResponse) | Params queries all parameters. | GET|/cosmos/auth/v1beta1/params|
| `ModuleAccounts` | [QueryModuleAccountsRequest](#cosmos.auth.v1beta1.QueryModuleAccountsRequest) | [QueryModuleAccountsResponse](#cosmos.auth.v1beta1.QueryModuleAccountsResponse) | ModuleAccounts returns all the existing module accounts. | GET|/cosmos/auth/v1beta1/module_accounts|
Expand Down Expand Up @@ -3354,7 +3362,7 @@ VersionInfo is the type for the GetNodeInfoResponse message.
| `build_tags` | [string](#string) | | |
| `go_version` | [string](#string) | | |
| `build_deps` | [Module](#cosmos.base.tendermint.v1beta1.Module) | repeated | |
| `cosmos_sdk_version` | [string](#string) | | |
| `cosmos_sdk_version` | [string](#string) | | @since Cosmos SDK 0.43 |



Expand Down Expand Up @@ -5950,6 +5958,8 @@ MsgVoteResponse defines the Msg/Vote response type.
### MsgVoteWeighted
MsgVoteWeighted defines a message to cast a vote.

@since Cosmos SDK 0.43


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
Expand All @@ -5967,6 +5977,8 @@ MsgVoteWeighted defines a message to cast a vote.
### MsgVoteWeightedResponse
MsgVoteWeightedResponse defines the Msg/VoteWeighted response type.

@since Cosmos SDK 0.43




Expand All @@ -5987,7 +5999,9 @@ Msg defines the bank Msg service.
| ----------- | ------------ | ------------- | ------------| ------- | -------- |
| `SubmitProposal` | [MsgSubmitProposal](#cosmos.gov.v1beta1.MsgSubmitProposal) | [MsgSubmitProposalResponse](#cosmos.gov.v1beta1.MsgSubmitProposalResponse) | SubmitProposal defines a method to create new proposal given a content. | |
| `Vote` | [MsgVote](#cosmos.gov.v1beta1.MsgVote) | [MsgVoteResponse](#cosmos.gov.v1beta1.MsgVoteResponse) | Vote defines a method to add a vote on a specific proposal. | |
| `VoteWeighted` | [MsgVoteWeighted](#cosmos.gov.v1beta1.MsgVoteWeighted) | [MsgVoteWeightedResponse](#cosmos.gov.v1beta1.MsgVoteWeightedResponse) | VoteWeighted defines a method to add a weighted vote on a specific proposal. | |
| `VoteWeighted` | [MsgVoteWeighted](#cosmos.gov.v1beta1.MsgVoteWeighted) | [MsgVoteWeightedResponse](#cosmos.gov.v1beta1.MsgVoteWeightedResponse) | VoteWeighted defines a method to add a weighted vote on a specific proposal.

@since Cosmos SDK 0.43 | |
| `Deposit` | [MsgDeposit](#cosmos.gov.v1beta1.MsgDeposit) | [MsgDepositResponse](#cosmos.gov.v1beta1.MsgDepositResponse) | Deposit defines a method to add deposit on a specific proposal. | |

<!-- end services -->
Expand Down
6 changes: 6 additions & 0 deletions proto/cosmos/auth/v1beta1/query.proto
View file
Open in desktop
Expand Up @@ -13,6 +13,8 @@ option go_package = "github.com/cosmos/cosmos-sdk/x/auth/types";
// Query defines the gRPC querier service.
service Query {
// Accounts returns all the existing accounts
//
// @since Cosmos SDK 0.43
rpc Accounts(QueryAccountsRequest) returns (QueryAccountsResponse) {
option (google.api.http).get = "/cosmos/auth/v1beta1/accounts";
}
Expand Down Expand Up @@ -49,12 +51,16 @@ service Query {
}

// QueryAccountsRequest is the request type for the Query/Accounts RPC method.
//
// @since Cosmos SDK 0.43
message QueryAccountsRequest {
// pagination defines an optional pagination for the request.
cosmos.base.query.v1beta1.PageRequest pagination = 1;
}

// QueryAccountsResponse is the response type for the Query/Accounts RPC method.
//
// @since Cosmos SDK 0.43
message QueryAccountsResponse {
// accounts are the existing accounts
repeated google.protobuf.Any accounts = 1 [(cosmos_proto.accepts_interface) = "AccountI"];
Expand Down
2 changes: 2 additions & 0 deletions proto/cosmos/base/query/v1beta1/pagination.proto
View file
Open in desktop
Expand Up @@ -32,6 +32,8 @@ message PageRequest {
bool count_total = 4;

// reverse is set to true if results are to be returned in the descending order.
//
// @since Cosmos SDK 0.43
Copy link
Contributor Author

Choose a reason for hiding this comment

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

other ideas that came up:

  • @since cosmos-sdk 0.43
  • @since github.com/cosmos/cosmos-sdk@v0.43 (but that's not actually a valid github or go module tag)
  • @since 0.43

Opinions?

Copy link
Member

Choose a reason for hiding this comment

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

Turns out there is a bit of a collision then using the JSDoc/TSDoc style @since. It is represented as if it's about the JS/TS library version. See https://jsdoc.app/tags-since.html The same problem would apply for types generated in Java.

Bildschirmfoto 2021-10-25 um 23 13 56

I think it's better to avoid this conflict and just make it plain text for the callers, starting with Since:.

Since: cosmos-sdk 0.43 would be my favourite. It is clean, refers to the repo name and allows us to split by whitespace when parsing.

Copy link
Member

Choose a reason for hiding this comment

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

I think that's actually okay if we stick to strict API based versioning - the npm or maven package version would line up with the go version. I know want to have a high level user friendly package. But for individual low level proto packages I think could make a lot of sense to have own npm package per proto package with the exact same API version. Then the @since tag would work nicely with existing tools.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's actually okay if we stick to strict API based versioning - the npm or maven package version would line up with the go version

Personally I actually don't see a lot of advantages for having the npm/maven/go/proto types versions all be the same. On the other hand, one disadvantage I see is the need to lock proto bumping speed with JS/Java/Rust bumping speed.

Simon mentioned yesterday about CosmJS using the long library for big integers. If CosmJS decides to change that to another lib (e.g. the native TypeScript bigint) which is a breaking change, they would need to wait for the proto pkg version to bump... which by design would take a long time.

Copy link
Member

Choose a reason for hiding this comment

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

While I agree with the idea of module specific packes that are close to Cosmos SDK versioning, enforcing the same version number across different client ecosystems seems to be very restricting. There can be breaking changes in the code generation that are not related to the API itself as Amury described. Some ecosystems structure in smaller packages, some in larger. Also note that the code generation does not necessarily happen in a type library. You can always add types in the application. Then you suddenly bind Cosmos SDK version to the version of the app.

bool reverse = 5;
}

Expand Down
1 change: 1 addition & 0 deletions proto/cosmos/base/tendermint/v1beta1/query.proto
View file
Open in desktop
Expand Up @@ -124,6 +124,7 @@ message VersionInfo {
string build_tags = 5;
string go_version = 6;
repeated Module build_deps = 7;
// @since Cosmos SDK 0.43
string cosmos_sdk_version = 8;
}

Expand Down
6 changes: 6 additions & 0 deletions proto/cosmos/gov/v1beta1/tx.proto
View file
Open in desktop
Expand Up @@ -18,6 +18,8 @@ service Msg {
rpc Vote(MsgVote) returns (MsgVoteResponse);

// VoteWeighted defines a method to add a weighted vote on a specific proposal.
//
// @since Cosmos SDK 0.43
rpc VoteWeighted(MsgVoteWeighted) returns (MsgVoteWeightedResponse);

// Deposit defines a method to add deposit on a specific proposal.
Expand Down Expand Up @@ -61,6 +63,8 @@ message MsgVote {
message MsgVoteResponse {}

// MsgVoteWeighted defines a message to cast a vote.
//
// @since Cosmos SDK 0.43
message MsgVoteWeighted {
option (gogoproto.equal) = false;
option (gogoproto.goproto_stringer) = false;
Expand All @@ -73,6 +77,8 @@ message MsgVoteWeighted {
}

// MsgVoteWeightedResponse defines the Msg/VoteWeighted response type.
//
// @since Cosmos SDK 0.43
message MsgVoteWeightedResponse {}

// MsgDeposit defines a message to submit a deposit to an existing proposal.
Expand Down
2 changes: 2 additions & 0 deletions types/query/pagination.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions x/auth/types/query.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions x/gov/types/tx.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.