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.

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: run make protogen #21378

Merged
merged 4 commits into from
Aug 23, 2024
Merged

chore: run make protogen #21378

merged 4 commits into from
Aug 23, 2024

Conversation

facundomedica
Copy link
Member

@facundomedica facundomedica commented Aug 21, 2024

Description

Closes: #XXXX


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title, you can find examples of the prefixes below:
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

  • New Features

    • Enhanced configuration management features added for improved interaction with the Go ecosystem.
  • Bug Fixes

    • Adjustments made to protobuf message structures, enhancing functionality and clarity.
  • Refactor

    • Streamlined dependencies and improved initialization processes for query and transaction protocols.
  • Documentation

    • Updated import paths to reflect new API structures, ensuring alignment with the latest versioning strategy.

@facundomedica facundomedica requested a review from a team as a code owner August 21, 2024 16:53
Copy link
Contributor

coderabbitai bot commented Aug 21, 2024

Walkthrough

Walkthrough

The changes reflect a significant refactor across several files in the Cosmos SDK, focusing on simplifying protocol buffer initializations, modifying message structures, and updating dependencies. Key adjustments include the removal of certain import statements, updates to protobuf message definitions, and enhancements to configuration settings. These alterations aim to improve code maintainability, streamline dependencies, and enhance integration within the Go ecosystem.

Changes

Files Change Summary
api/cosmos/consensus/v1/query.pulsar.go Removed consensus protocol initialization; modified byte array for protobuf descriptor, suggesting structural changes.
api/cosmos/consensus/v1/tx.pulsar.go Removed cometbft/abci/v1 import; altered message structure with modified field definitions to improve clarity.
depinject/internal/appconfig/buf.gen.pulsar.yaml Added managed section with enabled set to true and a go_package_prefix, enhancing configuration management.
depinject/internal/appconfig/testpb/test.pulsar.go Updated import path for dependencies; modified raw descriptor byte array to reflect new protobuf definitions.
depinject/go.mod Added new dependency for cosmossdk.io/api at version v0.7.5, enhancing module functionality.
depinject/internal/appconfiggogo/buf.gen.gogo.yaml Updated opt field to include additional module path mapping for improved plugin functionality.
testutil/x/counter/proto/buf.gen.gogo.yaml Updated opt field with a new module path mapping, enhancing integration with the Cosmos SDK.

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e88c138 and 6d53955.

Files ignored due to path filters (7)
  • depinject/internal/appconfig/buf.lock is excluded by !**/*.lock
  • depinject/internal/appconfiggogo/buf.lock is excluded by !**/*.lock
  • depinject/internal/appconfiggogo/testpb/test.pb.go is excluded by !**/*.pb.go
  • testutil/x/counter/types/module.pb.go is excluded by !**/*.pb.go
  • x/consensus/types/query.pb.go is excluded by !**/*.pb.go
  • x/consensus/types/tx.pb.go is excluded by !**/*.pb.go
  • x/staking/types/staking.pb.go is excluded by !**/*.pb.go
Files selected for processing (4)
  • api/cosmos/consensus/v1/query.pulsar.go (2 hunks)
  • api/cosmos/consensus/v1/tx.pulsar.go (2 hunks)
  • depinject/internal/appconfig/buf.gen.pulsar.yaml (1 hunks)
  • depinject/internal/appconfig/testpb/test.pulsar.go (2 hunks)
Additional context used
Path-based instructions (3)
api/cosmos/consensus/v1/query.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

api/cosmos/consensus/v1/tx.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

depinject/internal/appconfig/testpb/test.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

Additional comments not posted (3)
depinject/internal/appconfig/buf.gen.pulsar.yaml (1)

2-5: Configuration Update Approved.

The addition of the managed section with enabled: true and go_package_prefix is a positive step towards better management of generated code.

depinject/internal/appconfig/testpb/test.pulsar.go (2)

5-5: Verify the new import path.

The import path has been updated to cosmossdk.io/api/cosmos/app/v1alpha1. Ensure that this path is correct and aligns with the intended version of the Cosmos SDK API.


2347-2354: Verify changes in the byte array descriptor.

The file_testpb_test_proto_rawDesc has been modified. Ensure that these changes are intentional and correctly reflect the updated protocol buffer definitions.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e88c138 and 6d53955.

Files ignored due to path filters (7)
  • depinject/internal/appconfig/buf.lock is excluded by !**/*.lock
  • depinject/internal/appconfiggogo/buf.lock is excluded by !**/*.lock
  • depinject/internal/appconfiggogo/testpb/test.pb.go is excluded by !**/*.pb.go
  • testutil/x/counter/types/module.pb.go is excluded by !**/*.pb.go
  • x/consensus/types/query.pb.go is excluded by !**/*.pb.go
  • x/consensus/types/tx.pb.go is excluded by !**/*.pb.go
  • x/staking/types/staking.pb.go is excluded by !**/*.pb.go
Files selected for processing (4)
  • api/cosmos/consensus/v1/query.pulsar.go (2 hunks)
  • api/cosmos/consensus/v1/tx.pulsar.go (2 hunks)
  • depinject/internal/appconfig/buf.gen.pulsar.yaml (1 hunks)
  • depinject/internal/appconfig/testpb/test.pulsar.go (2 hunks)
Additional context used
Path-based instructions (3)
api/cosmos/consensus/v1/query.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

api/cosmos/consensus/v1/tx.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

depinject/internal/appconfig/testpb/test.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

Additional comments not posted (6)
depinject/internal/appconfig/buf.gen.pulsar.yaml (1)

2-5: Ensure compatibility with Go package prefix.

The managed section has been added with enabled: true and a default go_package_prefix. Verify that this prefix aligns with the project's package structure and does not conflict with existing packages.

api/cosmos/consensus/v1/query.pulsar.go (1)

Line range hint 15-17: Verify removal of initialization call.

The call to file_cosmos_consensus_v1_consensus_proto_init() has been removed from file_cosmos_consensus_v1_query_proto_init(). Ensure that this change does not affect the initialization of dependencies and that the removed function is no longer required.

api/cosmos/consensus/v1/tx.pulsar.go (2)

3-3: Verify impact of import statement removal.

The import statement for the CometBFT ABCI protocol buffer has been removed. Ensure that this does not affect any dependencies or functionality that rely on this import.


1416-1474: Review byte array restructuring for readability.

The byte array representing the raw descriptor has been restructured. Verify that the changes improve readability and maintain the same functionality.

depinject/internal/appconfig/testpb/test.pulsar.go (2)

5-5: Verify the new import path.

The import path has been updated. Ensure that cosmossdk.io/api/cosmos/app/v1alpha1 is the correct and intended path for the dependencies used in this file.


2347-2354: Verify protocol buffer definition changes.

The byte array content has changed, indicating updates to the protocol buffer definitions. Ensure that these changes are intentional and that they maintain compatibility with the existing system.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e88c138 and 6d53955.

Files ignored due to path filters (7)
  • depinject/internal/appconfig/buf.lock is excluded by !**/*.lock
  • depinject/internal/appconfiggogo/buf.lock is excluded by !**/*.lock
  • depinject/internal/appconfiggogo/testpb/test.pb.go is excluded by !**/*.pb.go
  • testutil/x/counter/types/module.pb.go is excluded by !**/*.pb.go
  • x/consensus/types/query.pb.go is excluded by !**/*.pb.go
  • x/consensus/types/tx.pb.go is excluded by !**/*.pb.go
  • x/staking/types/staking.pb.go is excluded by !**/*.pb.go
Files selected for processing (4)
  • api/cosmos/consensus/v1/query.pulsar.go (2 hunks)
  • api/cosmos/consensus/v1/tx.pulsar.go (2 hunks)
  • depinject/internal/appconfig/buf.gen.pulsar.yaml (1 hunks)
  • depinject/internal/appconfig/testpb/test.pulsar.go (2 hunks)
Additional context used
Path-based instructions (3)
api/cosmos/consensus/v1/query.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

api/cosmos/consensus/v1/tx.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

depinject/internal/appconfig/testpb/test.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

Additional comments not posted (3)
depinject/internal/appconfig/buf.gen.pulsar.yaml (1)

2-5: Configuration changes approved.

The addition of the managed section with enabled: true and go_package_prefix: default: cosmossdk.io/api is correct. This change enhances the configuration by enabling management features and specifying a default package prefix for Go, improving integration and organization.

depinject/internal/appconfig/testpb/test.pulsar.go (2)

5-5: Verify the correctness of the new import path.

The import path has been updated to cosmossdk.io/api/cosmos/app/v1alpha1. Ensure that this path is correct and aligns with the intended API structure or versioning strategy.

Run the following script to check for the existence of the new import path:

Verification successful

Import path verified successfully.

The new import path cosmossdk.io/api/cosmos/app/v1alpha1 is present in multiple files across the codebase, indicating it is correctly integrated and aligns with the intended API structure or versioning strategy.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of the new import path in the codebase.

# Test: Search for the new import path. Expect: At least one occurrence.
rg --type go 'cosmossdk.io/api/cosmos/app/v1alpha1'

Length of output: 3119


2347-2354: Verify the impact of raw descriptor byte array changes.

The raw descriptor byte array has been updated. Ensure that these changes do not negatively impact serialization and deserialization processes.

Run the following script to check for any references to file_testpb_test_proto_rawDesc and ensure compatibility:

Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

Looks like you are missing an override in the buf.yaml.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6d53955 and 7a9c5ae.

Files ignored due to path filters (3)
  • depinject/go.sum is excluded by !**/*.sum
  • depinject/internal/appconfiggogo/testpb/test.pb.go is excluded by !**/*.pb.go
  • testutil/x/counter/types/module.pb.go is excluded by !**/*.pb.go
Files selected for processing (3)
  • depinject/go.mod (1 hunks)
  • depinject/internal/appconfiggogo/buf.gen.gogo.yaml (1 hunks)
  • testutil/x/counter/proto/buf.gen.gogo.yaml (1 hunks)
Additional comments not posted (3)
depinject/internal/appconfiggogo/buf.gen.gogo.yaml (1)

5-5: Configuration enhancement approved.

The addition of the mapping for Mcosmos/app/v1alpha1/module.proto to cosmossdk.io/api/cosmos/app/v1alpha1 in the opt parameter enhances the plugin's functionality.

testutil/x/counter/proto/buf.gen.gogo.yaml (1)

5-5: Configuration enhancement approved.

The addition of the mapping for Mcosmos/app/v1alpha1/module.proto to cosmossdk.io/api/cosmos/app/v1alpha1 in the opt parameter enhances the plugin's functionality.

depinject/go.mod (1)

6-6: Dependency addition approved.

The addition of the cosmossdk.io/api v0.7.5 dependency enhances the module's capabilities by incorporating additional functionalities or types.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6d53955 and 7a9c5ae.

Files ignored due to path filters (3)
  • depinject/go.sum is excluded by !**/*.sum
  • depinject/internal/appconfiggogo/testpb/test.pb.go is excluded by !**/*.pb.go
  • testutil/x/counter/types/module.pb.go is excluded by !**/*.pb.go
Files selected for processing (3)
  • depinject/go.mod (1 hunks)
  • depinject/internal/appconfiggogo/buf.gen.gogo.yaml (1 hunks)
  • testutil/x/counter/proto/buf.gen.gogo.yaml (1 hunks)
Additional comments not posted (3)
depinject/internal/appconfiggogo/buf.gen.gogo.yaml (1)

5-5: Verify the impact of the new path mapping.

The addition of the path mapping in the opt field may affect how the gocosmos plugin processes proto files. Ensure that this change is compatible with existing configurations and does not introduce any unintended side effects.

testutil/x/counter/proto/buf.gen.gogo.yaml (1)

5-5: Verify the impact of the new module path.

The addition of the module path in the opt field may affect how the gocosmos plugin interacts with the specified modules. Ensure that this change aligns with project requirements and does not introduce any conflicts.

depinject/go.mod (1)

6-6: Verify compatibility of the new dependency.

The addition of cosmossdk.io/api v0.7.5 as a dependency may introduce new features or capabilities. Ensure that this dependency is compatible with existing ones and does not cause any version conflicts.

depinject/go.mod Outdated
@@ -3,6 +3,7 @@ module cosmossdk.io/depinject
go 1.20

require (
cosmossdk.io/api v0.7.5
Copy link
Member

Choose a reason for hiding this comment

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

Depinject shouldn't import api.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess it was because of this:

--	_ "cosmossdk.io/depinject/appconfig/v1alpha1"
++	_ "cosmossdk.io/api/cosmos/app/v1alpha1"

Not sure what is going on with these proto things, how they can stop working just like that

Copy link
Member

Choose a reason for hiding this comment

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

We bumped buf in proto-builder 0.15, so that could be that: contrib/Dockerfile

Copy link
Member Author

@facundomedica facundomedica Aug 23, 2024

Choose a reason for hiding this comment

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

I don't know how to fix this, the internal protos are using cosmos.app.v1alpha1.module. EDIT: nvm

Copy link
Member Author

Choose a reason for hiding this comment

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

@julienrbrt fixed, pls re-review

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7a9c5ae and 2936749.

Files selected for processing (3)
  • depinject/internal/appconfig/buf.gen.pulsar.yaml (1 hunks)
  • depinject/internal/appconfig/testpb/test.pulsar.go (1 hunks)
  • depinject/internal/appconfiggogo/buf.gen.gogo.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • depinject/internal/appconfig/buf.gen.pulsar.yaml
  • depinject/internal/appconfig/testpb/test.pulsar.go
  • depinject/internal/appconfiggogo/buf.gen.gogo.yaml

@@ -2,7 +2,7 @@ version: v1
plugins:
- name: gocosmos
out: ..
opt: plugins=grpc,Mgoogle/protobuf/any.proto=github.com/cosmos/gogoproto/types/any
opt: plugins=grpc,Mgoogle/protobuf/any.proto=github.com/cosmos/gogoproto/types/any,Mcosmos/app/v1alpha1/module.proto=cosmossdk.io/api/cosmos/app/v1alpha1
Copy link
Member

Choose a reason for hiding this comment

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

We may need to backport this PR to 0.52 only for this 😅

@julienrbrt julienrbrt added this pull request to the merge queue Aug 23, 2024
Merged via the queue into main with commit 7c69f65 Aug 23, 2024
78 checks passed
@julienrbrt julienrbrt deleted the facu/protogen branch August 23, 2024 19:24
@julienrbrt
Copy link
Member

@Mergifyio backport release/0.52.x

Copy link
Contributor

mergify bot commented Aug 23, 2024

backport release/0.52.x

❌ No backport have been created

  • Backport to branch release/0.52.x failed

GitHub error: Branch not found

@julienrbrt
Copy link
Member

@Mergifyio backport release/v0.52.x

Copy link
Contributor

mergify bot commented Aug 23, 2024

backport release/v0.52.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Aug 23, 2024
(cherry picked from commit 7c69f65)

# Conflicts:
#	api/cosmos/consensus/v1/query.pulsar.go
#	api/cosmos/consensus/v1/tx.pulsar.go
#	depinject/internal/appconfig/buf.gen.pulsar.yaml
#	depinject/internal/appconfig/testpb/test.pulsar.go
#	depinject/internal/appconfiggogo/buf.gen.gogo.yaml
@mergify mergify bot mentioned this pull request Aug 23, 2024
12 tasks
julienrbrt added a commit that referenced this pull request Aug 23, 2024
Co-authored-by: Facundo Medica <[email protected]>
Co-authored-by: Julien Robert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants