-
Notifications
You must be signed in to change notification settings - Fork 699
chore: remove orderedmap #8555
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: remove orderedmap #8555
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8555 +/- ##
===========================================
+ Coverage 17.66% 58.00% +40.33%
===========================================
Files 28 317 +289
Lines 2711 22743 +20032
===========================================
+ Hits 479 13191 +12712
- Misses 2205 8948 +6743
- Partials 27 604 +577
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Great PR. Robust implementation of ser/de.
Approving this as the only comment I have is a non blocker.
@@ -89,6 +89,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket(ctx sdk.Context, sourceClient st | |||
return im.app.OnAcknowledgementPacket(ctx, sourceClient, destinationClient, sequence, acknowledgement, payload, relayer) | |||
} | |||
|
|||
// TODO: Something looks off about this, please review carefully |
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.
Will you do this in this pr?
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.
No, I just added it because I happened to come across it.
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.
approved modulo codecov
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.
Pull Request Overview
This PR removes the orderedmap
dependency and refactors the packet-forward middleware to use native maps and time.Duration
, updating associated tests and usage across the codebase.
- Eliminated all imports and usage of
github.com/iancoleman/orderedmap
. - Refactored
PacketMetadata
/ForwardMetadata
to use native Go types and helper methods (ToMap
,ToMemo
). - Updated tests and IBC middleware to use the new
ToMemo
API andtime.Duration
instead of custom JSON wrappers.
Reviewed Changes
Copilot reviewed 18 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
simapp/go.mod & modules/**/go.mod | Removed orderedmap from module dependencies |
modules/apps/rate-limiting/v2/ibc_middleware.go | Minor TODO addition |
modules/apps/packet-forward-middleware/types/keys.go | Added new metadata key constants |
modules/apps/packet-forward-middleware/types/forward.go | Removed JSONObject /orderedmap, implemented native maps |
modules/apps/packet-forward-middleware/types/forward_test.go | Expanded tests for new API and increased coverage |
modules/apps/packet-forward-middleware/keeper/*.go & tests | Adjusted ForwardTransferPacket signature and memo handling |
modules/apps/packet-forward-middleware/ibc_middleware.go | Updated OnRecvPacket to use GetPacketMetadataFromPacketdata |
e2e/tests/packet_forward_middleware/**/*.go | Updated E2E tests to use ToMemo and removed orderedmap use |
go.mod | Removed orderedmap globally |
Comments suppressed due to low confidence (2)
modules/apps/packet-forward-middleware/types/forward.go:35
- The Validate method calls errors.New but the standard "errors" package is not imported. Add
import "errors"
or replace with errorsmod.New.
func (m ForwardMetadata) Validate() error {
modules/apps/packet-forward-middleware/types/forward.go:84
- json.Marshal is used but the "encoding/json" package is not imported. Add
import "encoding/json"
.
packetMetadataJSON, err := json.Marshal(packetMetadataMap)
Description
closes: IBCGO-2338
closes: #8378
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/
) if anything is changed.godoc
comments if relevant.Files changed
in the GitHub PR explorer.