-
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
chore(types): deprecate acc address stringer #21435
Conversation
WalkthroughWalkthroughThe changes introduce a deprecation notice for the Changes
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 Documentation and Community
|
The linter will be annoying here, but it's for our own good :D |
Co-authored-by: Julián Toledano <[email protected]>
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: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (4)
- CHANGELOG.md (1 hunks)
- Makefile (1 hunks)
- types/address.go (9 hunks)
- types/config.go (2 hunks)
Files skipped from review due to trivial changes (1)
- Makefile
Additional context used
Path-based instructions (3)
types/config.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.types/address.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.CHANGELOG.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (11)
types/config.go (2)
13-18
: FunctionKeyringServiceName
is implemented correctly.The logic to dynamically determine the keyring service name based on
version.Name
with a fallback toDefaultKeyringServiceName
is clear and follows good coding practices.
21-22
: Updated comment onConfig
struct is clear and informative.The deprecation notice and guidance for transitional use are well-documented, aiding in a smooth transition for users.
types/address.go (8)
287-287
: Deprecation ofAccAddress.String()
is appropriate.The shift towards using
address.Codec
for address conversions is a positive move towards centralization and standardization.
194-194
: Deprecation ofAccAddressFromBech32
is well-justified.Encouraging the use of
address.Codec
for conversions aligns with the goals of centralizing and standardizing address handling.
183-183
: Deprecation ofMustAccAddressFromBech32
is correctly implemented.This change supports the transition to a more robust and centralized method for address conversion.
441-441
: Deprecation ofValAddress.String()
is well-placed.Moving towards
address.Codec
for conversions is a strategic improvement in address handling.
335-335
: Deprecation ofValAddressFromBech32
is justified.Encouraging the use of
address.Codec
for conversions aligns with the goals of centralizing and standardizing address handling.
344-344
: Deprecation ofMustValAddressFromBech32
is correctly implemented.This change supports the transition to a more robust and centralized method for address conversion.
590-590
: Deprecation ofConsAddress.String()
is appropriate.The shift towards using
address.Codec
for address conversions is a positive move towards centralization and standardization.
490-490
: Deprecation ofConsAddressFromBech32
is well-justified.Encouraging the use of
address.Codec
for conversions aligns with the goals of centralizing and standardizing address handling.CHANGELOG.md (1)
55-58
: Changelog update is clear and well-documented.The addition of the "Deprecated" section clearly communicates the deprecation of the
String()
method forAccAddress
,ValAddress
, andConsAddress
. The explanation ties the deprecation to the use of the outdatedsdk.Config
, guiding users towards the neweraddress.Codec
approach. The link to the pull request provides a direct reference for further details.The changes to the changelog are approved.
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: 0
Co-authored-by: Julián Toledano <[email protected]> (cherry picked from commit 39b61a3) # Conflicts: # CHANGELOG.md
Co-authored-by: Julien Robert <[email protected]>
Description
ref: #21430 (comment)
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...
!
in the type prefix if API or client breaking changeCHANGELOG.md
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...
Summary by CodeRabbit
New Features
Deprecations
String()
method forAccAddress
,ValAddress
, andConsAddress
, urging users to use theaddress.Codec
for address conversions.Documentation