docs: update protobuf links #22182
docs: update protobuf links #22182
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request primarily involve updates to documentation across various files related to Protocol Buffers within the Cosmos SDK. Key modifications include updating URLs for references to the Protocol Buffers documentation, enhancing clarity around encoding processes, and refining guidelines for module developers. These updates aim to ensure that the documentation remains current and accurately reflects the latest practices and standards without altering any code logic or functionality. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (28)
docs/build/building-modules/02-messages-and-queries.md (1)
100-100: Approved: Good update to Protobuf services referenceThe addition of a specific link to the Protobuf services documentation is a valuable improvement. It provides developers with a direct resource for understanding how to define queries using Protobuf services.
Consider adding a brief explanation of why Protobuf services are recommended for defining queries in Cosmos SDK modules. This could help developers understand the benefits and rationale behind this approach.
docs/architecture/adr-044-protobuf-updates-guidelines.md (3)
Line range hint
33-62: LGTM: Clear guidelines for adding new Protobuf definitions.This new section provides valuable guidance for module developers when adding new Protobuf definitions. The requirement for the "Since:" comment is well-explained, and the examples of valid and invalid comments are particularly helpful.
Consider adding a brief explanation of why this comment format is important, such as its role in helping client libraries manage version-specific features.
Line range hint
76-87: LGTM: Comprehensive explanation of handling deprecated fields.The expanded section on deprecated fields provides clear guidance on how these fields can be handled, including the possibility of protocol-breaking changes. The examples of upgrade plans and governance split votes effectively illustrate the practical application of this guideline.
Consider adding a note about the importance of clearly communicating any behavior changes related to deprecated fields in release notes or upgrade guides.
Line range hint
95-102: Consider adding more context to the TODO section.While it's understandable that this section is still under development, it would be helpful to provide more context. Consider adding:
- A brief explanation of why these topics are important.
- Any preliminary thoughts or directions on these topics.
- A tentative timeline or process for addressing these TODOs.
This additional information would give readers a better understanding of the upcoming work and its significance.
docs/architecture/adr-031-msg-service.md (1)
178-178: LGTM: Content accurately reflects ADR changes.The text correctly describes the simplification of module functionality exposure. The use of "any more" as two words is grammatically correct in this formal context, though "anymore" would also be acceptable in less formal writing.
Consider using "anymore" for consistency if it's used elsewhere in the document, but the current form is also correct.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~178-~178: Did you mean the adverb “anymore”?
Context: ...n't need to create handlers and keepers any more. Use of Protocol Buffer auto-generated ...(ANY_MORE)
[uncategorized] ~178-~178: Did you mean the adverb “anymore”?
Context: ...ka handlers and keepers) is not exposed any more. A module interface can be seen as a bl...(ANY_MORE)
docs/architecture/adr-023-protobuf-naming.md (4)
Line range hint
41-45: Clarification of naming convention goals approvedThe added text effectively clarifies the objectives of the naming conventions, emphasizing user experience and the balance between conciseness and descriptiveness. This addition provides valuable context for the decisions made in this ADR.
Consider a minor grammatical improvement:
-The goal of this ADR is to provide thoughtful naming conventions that: +The goals of this ADR are to provide thoughtful naming conventions that:This change ensures subject-verb agreement, as multiple goals are listed.
Line range hint
66-69: Emphasis on concise naming approvedThe updates effectively stress the importance of concise yet descriptive names, providing clear guidance with practical examples. This addition enhances the document's usefulness for developers following these conventions.
For improved clarity, consider adding a brief explanation of why conciseness is particularly important in this context. For example:
Such conciseness makes names both more pleasant to work with and take up less -space within transactions and on the wire. +space within transactions and on the wire, which can have performance implications in a blockchain context.This addition would help readers understand the practical importance of concise naming in this specific domain.
Line range hint
89-117: Versioning guidelines approvedThe added guidelines on stable and unstable package versions provide crucial information for package maintainers. The emphasis on careful consideration before using alpha or beta versions is particularly valuable and aligns with best practices in software development.
For additional clarity, consider adding a brief explanation of the potential consequences of misusing alpha or beta versions. For example:
_`alpha` and `beta` should not be used to avoid responsibility for maintaining compatibility._ Whenever code is released into the wild, especially on a blockchain, there is a high cost to changing things. In some cases, for instance with immutable smart contracts, a breaking change may be impossible to fix. +Misuse of alpha or beta versions can lead to unexpected breaking changes, difficulty in upgrading, and potential loss of trust from users.This addition would reinforce the importance of proper version management and its impact on the ecosystem.
Line range hint
153-157: Package naming recommendations approvedThe updated recommendations for shorter top-level package names and limited sub-package depth align well with the principle of conciseness established earlier in the document. The specific examples for the Cosmos SDK provide clear and actionable guidance for implementation.
For completeness, consider adding a brief note about potential exceptions to these rules. For example:
For the Cosmos SDK, it is recommended that we simply write `cosmos.bank`, `cosmos.gov`, etc. rather than `cosmos.x.bank`. In practice, most non-module types can go straight in the `cosmos` package or we can introduce a `cosmos.base` package if needed. Note that this naming _will not_ change go package names, i.e. the `cosmos.bank` protobuf package will still live in `x/bank`. +There may be rare cases where deeper package structures are necessary for very complex modules or to maintain backwards compatibility. These should be carefully considered and documented.This addition would acknowledge potential edge cases while still emphasizing the general rule.
docs/learn/advanced/05-encoding.md (5)
Line range hint
1-33: LGTM! Clear explanation of encoding practices.The updates to the introduction and encoding section provide a clear and accurate explanation of the current encoding practices in the Cosmos SDK. The transition from
go-aminotogogoprotobufis well-explained, and the limited use of Amino is correctly described.Consider adding a brief explanation of why
gogoprotobufwas chosen over the standard Protocol Buffers implementation to provide more context for the readers.
Line range hint
55-68: LGTM! Comprehensive guidelines for protobuf message definitions.The updated guidelines provide clear and detailed instructions for defining protobuf messages, especially when dealing with interfaces. The examples and annotations are accurate and helpful.
Consider adding a brief explanation of why these specific annotations (
cosmos_proto.accepts_interfaceandcosmos_proto.implements_interface) are important in the context of the Cosmos SDK. This would help developers understand the rationale behind these guidelines.
Line range hint
93-207: LGTM! Comprehensive explanation of interface encoding withAny.The expanded section on Interface Encoding and Usage of
Anyprovides a thorough and clear explanation of how to handle variable-typed fields in protobuf messages. The examples and step-by-step guidance are particularly helpful for developers.Consider adding a brief note on the performance implications of using
Anyfor interface encoding. This would help developers make informed decisions when designing their data structures.
Line range hint
209-241: LGTM! Clear explanation ofAny's TypeURL handling in Cosmos SDK.The section provides crucial information about the differences in TypeURL handling between the Cosmos SDK and other implementations. The guidance on using helper functions from
"github.com/cosmos/cosmos-proto/anyutil"is clear and helpful.Consider adding a brief explanation of why the Cosmos SDK chose not to include the
type.googleapis.comprefix in TypeURLs. This would provide more context for the design decision.
Line range hint
243-246: LGTM! Helpful guidance on module creation and naming conventions.The updated FAQ section provides valuable information for module developers on using protobuf encoding and following naming conventions. The links to industry guidelines are excellent resources.
Consider adding a brief example of how these naming conventions might be applied in a typical Cosmos SDK module. This would help developers better understand how to implement these guidelines in practice.
docs/architecture/adr-019-protobuf-state-encoding.md (2)
Line range hint
1-1: Code example update approved with suggestionThe code example for marshaling evidence has been correctly updated to use
cdc.MarshalInterface(e), which aligns with the ADR's description of usingAnyto encode interfaces. This change improves the consistency and accuracy of the document.Consider adding a comment above the
MarshalEvidencefunction to explain that it's using the newMarshalInterfacemethod to handle interface types, which might help readers understand the change better.
Line range hint
1-1: Code example update approved with minor correction neededThe code example for unmarshaling evidence has been correctly updated to use
cdc.UnmarshalInterface(&evi, bz), which aligns with the ADR's description of usingAnyto decode interfaces. This change improves the consistency and accuracy of the document.There's a minor issue in the error return of the
UnmarshalEvidencefunction. The current implementation returnserr, nil, but it should beevi, errto match the function signature. Please update the return statement as follows:return evi, errdocs/learn/beginner/00-app-anatomy.md (8)
Line range hint
184-211: Improved clarity inMsgandQueryservices explanationThe updates to this section provide a clearer explanation of how
MsgandQueryservices work in Cosmos SDK modules. The changes accurately reflect the current implementation using Protobuf services.Some suggestions for further improvement:
- Consider adding a brief explanation of how
sdk.Msginterface is related to Protobuf message types.- It might be helpful to provide a small code example showing how a
Msgservice method is defined in Protobuf.
Line range hint
211-224: Enhanced explanation of gRPC Query ServicesThe updates provide a more detailed and accurate description of how gRPC Query services are implemented in Cosmos SDK modules. The explanation of the relationship between
query.proto,QueryServerinterface, and the module'skeeperis particularly clear.One suggestion for improvement:
- Consider adding a brief example of how a gRPC query endpoint is defined in
query.prototo illustrate the concept more concretely.
Line range hint
226-241: Updated Keeper sectionThe changes in this section are minimal but improve the clarity of the explanation. The updated text accurately describes the role of
keepersin Cosmos SDK modules.One suggestion:
- Consider adding a brief explanation of how the object-capabilities model enhances security in the context of
keepers.
Line range hint
243-258: Improved CLI sectionThe updates to the CLI section provide a clearer explanation of how CLI commands are organized and implemented in Cosmos SDK modules. The distinction between transaction and query commands is well explained.
Suggestion for improvement:
- Consider adding a brief example of how a typical CLI command is structured, perhaps showing a simplified version of a transaction or query command.
Line range hint
260-273: Enhanced gRPC sectionThe updated gRPC section provides a more comprehensive explanation of how gRPC is used in Cosmos SDK modules. The addition of information about service methods and the
RegisterGRPCGatewayRoutesmethod is particularly useful.One suggestion:
- Consider adding a brief example of how a gRPC service method is defined in Protobuf to illustrate the concept more concretely.
Line range hint
275-285: Improved gRPC-gateway REST Endpoints sectionThe updates to this section provide a clearer explanation of how gRPC-gateway REST endpoints are implemented in Cosmos SDK modules. The addition of information about Swagger definition files is particularly useful.
Suggestion:
- Consider adding a brief example of how a REST endpoint is defined using Protobuf annotations to illustrate the concept more concretely.
Line range hint
287-314: Updated Application Interface sectionThe changes in this section improve the clarity of the explanation of the application's command-line interface. The description of the main components (main function, query commands, and transaction commands) is accurate and well-structured.
One suggestion:
- Consider adding a brief example of how a typical command is added to the root command in the
main()function to illustrate the concept more concretely.
Line range hint
316-341: Updated Dependencies and Makefile sectionThe changes in this section accurately reflect the current best practices for managing dependencies and building Cosmos SDK applications. The explanation of
go.modand Makefile usage is clear and informative.Suggestion:
- Consider adding a brief explanation of why
go.modis preferred over other dependency management tools for Cosmos SDK applications.docs/architecture/adr-054-semver-compatible-modules.md (4)
Line range hint
13-19: LGTM: Abstract provides a clear overview of the ADR's purpose.The revised abstract effectively outlines the challenges and goals of the ADR. It provides a good foundation for readers to understand the importance of the proposed changes.
Consider adding a brief mention of the potential impact on developers or the ecosystem to further emphasize the significance of this ADR.
Line range hint
23-40: LGTM: Context section provides comprehensive background.The expanded context section effectively outlines the key problems that need to be addressed, providing readers with a solid understanding of the challenges involved in implementing semantic versioning for SDK modules.
Consider adding a brief example or use case to illustrate how these problems manifest in real-world scenarios. This could help readers better grasp the practical implications of the issues discussed.
Line range hint
323-463: LGTM: Solutions section comprehensively covers potential approaches.The addition of Approach E and the revisions to existing approaches provide a thorough exploration of potential solutions. This comprehensive analysis is crucial for informed decision-making.
Consider adding a summary table at the end of the Solutions section that compares the key aspects of each approach (e.g., complexity, backward compatibility, performance impact). This could help readers quickly grasp the trade-offs between different solutions.
Line range hint
485-494: LGTM: References section provides valuable related resources.The updated References section includes relevant links to discussions and documents, which is helpful for readers who want to explore the topic further.
Consider adding dates to the GitHub discussions and issues links to provide context on when these conversations took place.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (12)
- codec/unknownproto/unknown_fields.go (2 hunks)
- docs/architecture/adr-019-protobuf-state-encoding.md (2 hunks)
- docs/architecture/adr-023-protobuf-naming.md (1 hunks)
- docs/architecture/adr-027-deterministic-protobuf-serialization.md (4 hunks)
- docs/architecture/adr-031-msg-service.md (3 hunks)
- docs/architecture/adr-044-protobuf-updates-guidelines.md (1 hunks)
- docs/architecture/adr-054-semver-compatible-modules.md (2 hunks)
- docs/build/building-modules/02-messages-and-queries.md (1 hunks)
- docs/build/building-modules/15-depinject.md (1 hunks)
- docs/learn/advanced/05-encoding.md (3 hunks)
- docs/learn/beginner/00-app-anatomy.md (2 hunks)
- docs/learn/beginner/02-query-lifecycle.md (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- codec/unknownproto/unknown_fields.go
🧰 Additional context used
📓 Path-based instructions (11)
docs/architecture/adr-019-protobuf-state-encoding.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/architecture/adr-023-protobuf-naming.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/architecture/adr-027-deterministic-protobuf-serialization.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/architecture/adr-031-msg-service.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/architecture/adr-044-protobuf-updates-guidelines.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/architecture/adr-054-semver-compatible-modules.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/build/building-modules/02-messages-and-queries.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/build/building-modules/15-depinject.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/05-encoding.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/beginner/00-app-anatomy.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/beginner/02-query-lifecycle.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/architecture/adr-027-deterministic-protobuf-serialization.md
[typographical] ~293-~293: Consider adding a comma.
Context: ...message fields, once a message is parsed there's no way of telling whether a field was...(IF_THERE_COMMA)
docs/architecture/adr-031-msg-service.md
[uncategorized] ~178-~178: Did you mean the adverb “anymore”?
Context: ...n't need to create handlers and keepers any more. Use of Protocol Buffer auto-generated ...(ANY_MORE)
[uncategorized] ~178-~178: Did you mean the adverb “anymore”?
Context: ...ka handlers and keepers) is not exposed any more. A module interface can be seen as a bl...(ANY_MORE)
docs/learn/beginner/02-query-lifecycle.md
[grammar] ~44-~44: Did you mean “building”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...to various languages. These tools allow to build gRPC clients easily. One such tool is ...(ALLOW_TO)
🔇 Additional comments (27)
docs/build/building-modules/15-depinject.md (1)
55-55: Improved clarity and documentation linkThe update to this note enhances clarity by explicitly stating that Pulsar is optional. Additionally, the link to the official
protoc-gen-godocumentation provides a valuable resource for readers. These changes improve the overall quality of the documentation.docs/architecture/adr-044-protobuf-updates-guidelines.md (2)
Line range hint
7-7: LGTM: Changelog updated appropriately.The new entry in the Changelog is clear, concise, and accurately reflects the changes made to the document. It provides valuable information about the evolution of the ADR.
Line range hint
89-93: LGTM: Clear prohibition on field renaming.The section effectively communicates the prohibition on renaming fields in Protobuf structs. The explanation of why this is not allowed, including the potential impact on clients and JSON representations, is clear and informative.
docs/architecture/adr-031-msg-service.md (2)
80-80: LGTM: Protobuf link updated correctly.The link to the Protobuf documentation has been updated to the current URL. This change aligns with the PR objective of updating Protobuf links to their latest versions.
199-199: LGTM: Protobuf link updated correctly.The link to the Protobuf documentation about defining services has been updated to the current URL. This change is in line with the PR objective of updating Protobuf links to their latest versions.
docs/architecture/adr-023-protobuf-naming.md (1)
14-14: Link update approvedThe updated link to the Protocol Buffers style guide is correct and points to the current official documentation. This change improves the document's accuracy and usefulness.
docs/learn/advanced/05-encoding.md (2)
Line range hint
35-39: LGTM! Clear explanation of Gogoproto usage.The updates to the Gogoproto section provide clear guidance on using Protobuf encoding in the Cosmos SDK. The distinction between Gogoproto and the official Google protobuf implementation is well-explained.
Line range hint
70-91: LGTM! Clear explanation of transaction encoding with code references.The updates to the Transaction Encoding section provide clear guidance on how transactions are encoded and decoded in the Cosmos SDK. The addition of code references and links to the
auth/txmodule implementation is particularly helpful for developers.docs/architecture/adr-027-deterministic-protobuf-serialization.md (12)
18-18: URL update approvedThe Protobuf documentation link has been correctly updated to the latest official URL. This change improves the document's accuracy and ensures readers are directed to the most up-to-date resource.
58-58: URL update approvedThe link to the Protobuf varint encoding documentation has been correctly updated. This change ensures that readers are directed to the most current and relevant information about varint encoding in Protobuf.
77-77: URL update approvedThe link to the Protobuf encoding documentation has been correctly updated. This change ensures that readers have access to the most current information about Protobuf encoding, which is crucial for understanding the context of this ADR.
82-82: URL update approvedThe link to the Protobuf default values documentation has been correctly updated. This change ensures that readers can easily access the most up-to-date information about default values in Protobuf, which is important for understanding the serialization rules discussed in this ADR.
85-85: URL update approvedThe link to the Protobuf packed encoding documentation has been correctly updated. This change ensures that readers can access the most current information about packed encoding in Protobuf, which is relevant to the serialization rules discussed in this ADR.
291-292: URL update and formatting improvement approvedThe link to the Protobuf field order documentation has been correctly updated and reformatted into a single line. This change not only ensures access to the most current information but also improves the readability of the Markdown source.
299-299: URL update approvedThe link to the Protobuf default values documentation has been correctly updated. This change ensures that readers can access the most up-to-date information about default values in Protobuf, which is crucial for understanding the context of this reference.
303-303: URL update approvedThe link to the Protobuf default values documentation has been correctly updated. This change ensures that readers can access the most current information about default values in Protobuf, which is important for understanding the parsing behavior described in this reference.
306-306: URL update approvedThe link to the Protobuf default values documentation has been correctly updated. This change ensures that readers can access the most up-to-date information about default values in Protobuf, which is relevant to the serialization behavior described in this reference.
309-309: URL update approvedThe link to the Protobuf default values documentation has been correctly updated. This change ensures that readers can access the most current information about default values for enums in Protobuf, which is important for understanding the serialization rules discussed in this ADR.
312-312: URL update approvedThe link to the Protobuf default values documentation has been correctly updated. This change ensures that readers can access the most up-to-date information about default values for message fields in Protobuf, which is relevant to the serialization rules discussed in this ADR.
Line range hint
1-312: Comprehensive documentation update approvedThis update consistently refreshes all Protobuf documentation links throughout the ADR, ensuring they point to the latest official resources. These changes significantly enhance the document's accuracy, maintainability, and value to readers by providing access to the most current Protobuf information. The consistency in updating all relevant links is commendable and aligns well with best practices in technical documentation maintenance.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~288-~288: Possible missing comma found.
Context: ...ialization order is an implementation detail and the details of any particular imple...(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~293-~293: Consider adding a comma.
Context: ...message fields, once a message is parsed there's no way of telling whether a field was...(IF_THERE_COMMA)
docs/architecture/adr-019-protobuf-state-encoding.md (3)
24-24: Link update approvedThe Proto3 specification link has been successfully updated to the current and correct URL. This change improves the document's accuracy and aligns with the PR's objective of updating protobuf links.
59-59: Protocol Buffers link update approvedThe Protocol Buffers link has been correctly updated to the current URL. This change enhances the document's accuracy and is in line with the PR's goal of updating protobuf links.
Line range hint
1-1: Overall assessment: Changes improve document accuracy and consistencyThe updates in this ADR successfully address the PR's objective of updating protobuf links. The changes to the code examples accurately reflect the improvements described in the document, particularly regarding the use of
Anyfor encoding interfaces. These modifications enhance the overall consistency and accuracy of the ADR.docs/architecture/adr-054-semver-compatible-modules.md (4)
Line range hint
5-5: LGTM: Changelog updated appropriately.The addition of the second draft date in the changelog is a good practice for tracking document revisions.
Line range hint
9-9: LGTM: Status accurately reflects the document's current state.The DRAFT status is appropriate as the document presents multiple approaches without a final decision.
Line range hint
465-467: LGTM: Decision section accurately reflects the current state.The updated Decision section clearly communicates that no final decision has been made and highlights the most serious alternatives under consideration. This transparency is valuable for readers and contributors.
Line range hint
1-494: Overall: Significant improvements to the ADR's comprehensiveness and clarity.This revision of ADR-054 has substantially enhanced its value in exploring solutions for semantic versioning compatibility in Cosmos SDK modules. The expanded context, detailed exploration of multiple approaches, and clear presentation of the current decision state provide a solid foundation for further discussion and eventual decision-making.
Key improvements:
- More comprehensive context and problem statement
- Addition of a new approach (Approach E)
- Refinement of existing approaches
- Clear indication of the current decision state
These changes will greatly assist the Cosmos SDK community in understanding and addressing the challenges of implementing semantic versioning for modules.
julienrbrt
added
the
backport/v0.52.x
(cherry picked from commit f48fac3)
Co-authored-by: Wukingbow <[email protected]>
Description
All redirects from protobuf domains are updated to the latest links
Summary by CodeRabbit