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

feat: extract ic-mgmt call transform #774

Merged
merged 2 commits into from
Nov 27, 2024
Merged

Conversation

peterpeterparker
Copy link
Member

@peterpeterparker peterpeterparker commented Nov 26, 2024

Motivation

It’s literally impossible to test the internal transform function used within the static create function of the ic-mgmt class. However, as we need to address an issue to support WASM installation with chunks (#773), we must extend it. That’s why this PR extracts the function into a module, allowing us to provide tests for the mapping.

Notes

We delegate the responsibility of calling the transform function to agent-js, as it is used internally there and as we are "just" implementing the interface exposed by agent-js. While it is not thoroughly covered at the moment in the agent, I submitted a PR #954 today that adds at least one assertion.

No particular entry in CHANGELOG. This is an internal refactoring.

Changes

  • Extract internal transform function to a utility.
  • JSdocs and tests.
  • Improve types of the function (expected interface and return type)

peterpeterparker and others added 2 commits November 26, 2024 21:01
Signed-off-by: David Dal Busco <[email protected]>
Copy link
Contributor

size-limit report 📦

Path Size
@dfinity/ckbtc 8.03 KB (0%)
@dfinity/cketh 3.68 KB (0%)
@dfinity/cmc 1.4 KB (0%)
@dfinity/ledger-icrc 4.17 KB (0%)
@dfinity/ledger-icp 15.43 KB (0%)
@dfinity/nns 36.68 KB (0%)
@dfinity/nns-proto 140.98 KB (0%)
@dfinity/sns 16.95 KB (0%)
@dfinity/utils 4.66 KB (0%)
@dfinity/ic-management 3.16 KB (+0.41% 🔺)

Copy link

@AntonioVentilii AntonioVentilii left a comment

Choose a reason for hiding this comment

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

LGTM! We discussed offline about any other ways, so agreed with you!

I have just one comment

@peterpeterparker peterpeterparker merged commit 41871fa into main Nov 27, 2024
10 checks passed
@peterpeterparker peterpeterparker deleted the feat/extract-transform branch November 27, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants