-
Notifications
You must be signed in to change notification settings - Fork 662
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: rename denomination_trace
event type to denomination
#6467
Conversation
WalkthroughThe recent updates focus on renaming and adjusting event-related terminology and functionality within the Changes
Possibly related issues
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: 0
Outside diff range and nitpick comments (2)
docs/docs/02-apps/01-transfer/05-events.md (2)
Line range hint
25-31
: Remove trailing spaces to adhere to Markdown best practices.-| fungible_token_packet | memo | \{memo\} | +| fungible_token_packet | memo | \{memo\} | -| denomination | trace_hash | \{hex_hash\} | +| denomination | trace_hash | \{hex_hash\} |Tools
Markdownlint
28-28: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
29-29: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
30-30: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
31-31: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
Line range hint
9-9
: Address the issue of multiple top-level headings in the same document.Consider restructuring the document or adjusting the heading levels to comply with best practices.
Tools
Markdownlint
28-28: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
29-29: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
30-30: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
31-31: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (3)
- docs/docs/02-apps/01-transfer/05-events.md (1 hunks)
- modules/apps/transfer/keeper/relay.go (1 hunks)
- modules/apps/transfer/types/events.go (1 hunks)
Additional context used
Path-based instructions (3)
modules/apps/transfer/types/events.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.docs/docs/02-apps/01-transfer/05-events.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"modules/apps/transfer/keeper/relay.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Markdownlint
docs/docs/02-apps/01-transfer/05-events.md
25-25: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
26-26: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
27-27: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
28-28: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
29-29: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
30-30: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
31-31: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
9-9: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
golangci-lint
modules/apps/transfer/keeper/relay.go
55-55: undefined: Keeper (typecheck)
186-186: undefined: Keeper (typecheck)
326-326: undefined: Keeper (typecheck)
341-341: undefined: Keeper (typecheck)
349-349: undefined: Keeper (typecheck)
393-393: undefined: Keeper (typecheck)
409-409: undefined: Keeper (typecheck)
427-427: undefined: Keeper (typecheck)
Additional comments not posted (3)
modules/apps/transfer/types/events.go (1)
9-9
: RenamingEventTypeDenomTrace
toEventTypeDenom
aligns with the PR's objective to simplify the event type name.docs/docs/02-apps/01-transfer/05-events.md (1)
31-31
: The documentation correctly reflects the renaming ofdenomination_trace
todenomination
for thetrace_hash
field.Tools
Markdownlint
31-31: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spacesmodules/apps/transfer/keeper/relay.go (1)
278-278
: The event type in theOnRecvPacket
function has been updated fromEventTypeDenomTrace
toEventTypeDenom
, which is consistent with the changes across the codebase.
@@ -275,7 +275,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t | |||
|
|||
ctx.EventManager().EmitEvent( | |||
sdk.NewEvent( | |||
types.EventTypeDenomTrace, | |||
types.EventTypeDenom, | |||
sdk.NewAttribute(types.AttributeKeyTraceHash, token.Denom.Hash().String()), | |||
sdk.NewAttribute(types.AttributeKeyDenom, voucherDenom), |
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.
probably should update this to be token.Denom
instead of voucherDenom
, otherwise it's redundant
0469d96
to
58811c3
Compare
sdk.NewAttribute(types.AttributeKeyTraceHash, traceHash), | ||
sdk.NewAttribute(types.AttributeKeyDenom, voucherDenom), | ||
types.EventTypeDenom, | ||
sdk.NewAttribute(types.AttributeKeyDenomHash, token.Denom.Hash().String()), |
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.
also added this attribute to replace TraceHash
one
@@ -28,7 +28,7 @@ slug: /apps/transfer/events | |||
| fungible_token_packet | amount | \{amount\} | | |||
| fungible_token_packet | success | \{ackSuccess\} | | |||
| fungible_token_packet | memo | \{memo\} | | |||
| denomination_trace | trace_hash | \{hex_hash\} | | |||
| denomination | trace_hash | \{hex_hash\} | |
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.
I am going to update this before merging.
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)
- docs/docs/02-apps/01-transfer/05-events.md (1 hunks)
- modules/apps/transfer/internal/events/events.go (1 hunks)
- modules/apps/transfer/keeper/relay.go (1 hunks)
- modules/apps/transfer/types/events.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- modules/apps/transfer/keeper/relay.go
- modules/apps/transfer/types/events.go
Additional context used
Path-based instructions (2)
docs/docs/02-apps/01-transfer/05-events.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"modules/apps/transfer/internal/events/events.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Markdownlint
docs/docs/02-apps/01-transfer/05-events.md
25-25: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
26-26: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
27-27: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
28-28: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
29-29: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
30-30: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
9-9: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
Additional comments not posted (2)
docs/docs/02-apps/01-transfer/05-events.md (1)
31-32
: Update documentation to reflect new event type naming.The documentation correctly reflects the renaming of
denomination_trace
todenomination
. This aligns with the PR's objectives to simplify the event type name.modules/apps/transfer/internal/events/events.go (1)
106-112
: Update function to reflect new event type naming.The function
EmitDenomEvent
correctly reflects the renaming of the event type fromdenomination_trace
todenomination
. This change is consistent with the PR's objectives and the function implementation is correct.
|
Description
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.Summary by CodeRabbit
Documentation
denomination_trace
todenomination
.Refactor
EmitDenomTraceEvent
function toEmitDenomEvent
.EventTypeDenomTrace
toEventTypeDenom
in relevant functions.