-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
chore(proto): fix comments #20679
chore(proto): fix comments #20679
Conversation
WalkthroughThe changes primarily involve renaming and correcting message types in Cosmos SDK's API and protocol buffer files. The affected components are related to querying configuration details and managing historical staking records. Specifically, the Changes
Sequence DiagramsNo sequence diagrams are generated as the changes are limited to renaming types and classes, without introducing new features or altering control flows. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (1)
x/staking/types/staking.pb.go
is excluded by!**/*.pb.go
Files selected for processing (4)
- api/cosmos/app/v1alpha1/query.pulsar.go (1 hunks)
- api/cosmos/staking/v1beta1/staking.pulsar.go (1 hunks)
- proto/cosmos/app/v1alpha1/query.proto (1 hunks)
- x/staking/proto/cosmos/staking/v1beta1/staking.proto (1 hunks)
Files skipped from review due to trivial changes (1)
- x/staking/proto/cosmos/staking/v1beta1/staking.proto
Additional context used
Path-based instructions (2)
api/cosmos/app/v1alpha1/query.pulsar.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.api/cosmos/staking/v1beta1/staking.pulsar.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (1)
api/cosmos/staking/v1beta1/staking.pulsar.go (1)
14687-14687
: The updated comment clearly describes the purpose and usage ofHistoricalRecord
. Good job on improving clarity and consistency in documentation.
@@ -18,7 +18,7 @@ service Query { | |||
// QueryConfigRequest is the Query/Config request type. | |||
message QueryConfigRequest {} | |||
|
|||
// QueryConfigRequest is the Query/Config response type. | |||
// QueryConfigResponse is the Query/Config response type. |
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.
Tip
Codebase Verification
The renaming of QueryConfigRequest
to QueryConfigResponse
is incomplete. The old type name QueryConfigRequest
still exists in the following locations:
proto/cosmos/app/v1alpha1/query.proto: rpc Config(QueryConfigRequest) returns (QueryConfigResponse) {
proto/cosmos/app/v1alpha1/query.proto:// QueryConfigRequest is the Query/Config request type.
proto/cosmos/app/v1alpha1/query.proto:message QueryConfigRequest {}
Please update these occurrences to ensure consistency.
Analysis chain
Renaming of QueryConfigRequest
to QueryConfigResponse
correctly reflects its usage as a response type.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all occurrences of the old type name `QueryConfigRequest` have been updated.
# Test: Search for the old type name. Expect: No occurrences of the old type name.
rg --type proto "QueryConfigRequest"
Length of output: 66
Script:
#!/bin/bash
# Description: Verify that all occurrences of the old type name `QueryConfigRequest` have been updated.
# Test: Search for the old type name in .proto files. Expect: No occurrences of the old type name.
rg "QueryConfigRequest" --glob "*.proto"
Length of output: 305
@@ -843,7 +843,7 @@ func (*QueryConfigRequest) Descriptor() ([]byte, []int) { | |||
return file_cosmos_app_v1alpha1_query_proto_rawDescGZIP(), []int{0} | |||
} | |||
|
|||
// QueryConfigRequest is the Query/Config response type. | |||
// QueryConfigResponse is the Query/Config response type. |
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.
Tip
Codebase Verification
The renaming of QueryConfigRequest
to QueryConfigResponse
was not fully implemented. The occurrences of QueryConfigRequest
in both query_grpc.pb.go
and query.pulsar.go
indicate that the type still exists and is used in several places. This suggests that QueryConfigRequest
and QueryConfigResponse
are distinct types, and the renaming might have been a misunderstanding.
api/cosmos/app/v1alpha1/query_grpc.pb.go
: TheQueryConfigRequest
type is used in theConfig
method as an input parameter, whileQueryConfigResponse
is used as the output.api/cosmos/app/v1alpha1/query.pulsar.go
: TheQueryConfigRequest
type is defined and used in various reflection and protobuf-related functions.
You need to review the context and ensure that the renaming was intended and correctly applied. If QueryConfigRequest
should have been renamed to QueryConfigResponse
, then all these occurrences need to be updated. Otherwise, the current state might be correct, and both types are indeed required.
Please verify the intended changes and update the code accordingly.
api/cosmos/app/v1alpha1/query_grpc.pb.go
api/cosmos/app/v1alpha1/query.pulsar.go
Analysis chain
Renaming of QueryConfigRequest
to QueryConfigResponse
in the generated Go code is consistent with the changes in the proto file and correctly reflects its usage as a response type.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all occurrences of the old type name `QueryConfigRequest` have been updated in the Go code.
# Test: Search for the old type name. Expect: No occurrences of the old type name.
rg --type go "QueryConfigRequest"
Length of output: 8736
Script:
#!/bin/bash
# Extract relevant portions of `query_grpc.pb.go` where `QueryConfigRequest` is mentioned.
rg --type go --context 5 "QueryConfigRequest" api/cosmos/app/v1alpha1/query_grpc.pb.go
# Extract relevant portions of `query.pulsar.go` where `QueryConfigRequest` is mentioned.
rg --type go --context 5 "QueryConfigRequest" api/cosmos/app/v1alpha1/query.pulsar.go
Length of output: 15521
* main: feat(tx): port simappv2 changes (#20648) build(deps): Bump github.com/spf13/cobra from 1.8.0 to 1.8.1 (#20682) chore(proto): fix comments (#20679) (serverv2/cometbft) Commands support both json and yaml output (#20670) fix(x/staking,x/auth): regenerate mock to fix test (#20684) docs: ADR 074: Msg v2 (#20618) fix: nested multisig signatures using CLI (#20438) chore: fix spelling errors (#20674) fix: align Dockerfile for build-push-action (#20672) fix: avoid build fail when make localnet-build-env (#20671) build(deps): Bump bufbuild/buf-setup-action from 1.32.2 to 1.33.0 (#20669) chore: make function comment match function names (#20666) chore(consensus): add cometInfo to consensus (#20615) chore: fix typos (#20662) fix: Properly parse json in the wait-tx command. (#20631) fix(sims): check before sending RotateConsPubKey (#20659) test(types/address): add unit tests for the file types/address.go (#20237)
Description
fix type/message names in comments.
Summary by CodeRabbit
QueryConfigRequest
toQueryConfigResponse
andHistorical
toHistoricalRecord
.