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: add wrapper subinvoke test case #1328

Merged
merged 4 commits into from
Oct 18, 2022
Merged

Conversation

Niraj-Kamdar
Copy link
Contributor

No description provided.

Comment on lines 4 to 10
export function add(args: Args_add): i32 {
let importedArgs: ImportedArgs_add = {
a: args.a,
b: args.b
}
return Add_Module.add(importedArgs).unwrap()
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why this change? I'd prefer to have a different output for invoke and subinvoke. It'll be easy to debug and spot some horrible bugs that would be passed safely due to a buggy client implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I changed it just to make it work and have a super simple example

I'd prefer to have a different output for invoke and subinvoke

Gotcha, if we just use the add method in the invoke and add 1 to that result, would that be enough for you? Meaning that the invoke method would return the sum of two numbers + 1, and the subinvoke would just return the sum of two numbers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that'd work. You can call it add_and_increament then

@dOrgJelli dOrgJelli merged commit 1f97380 into origin-dev Oct 18, 2022
@Niraj-Kamdar
Copy link
Contributor Author

@dOrgJelli I had requested changes that weren't addressed yet. @cbrzn Can you create a patch PR for this?

@cbrzn
Copy link
Contributor

cbrzn commented Oct 19, 2022

@Niraj-Kamdar yah no worries is on my todo list

@dOrgJelli dOrgJelli deleted the nk/wrapper-subinvoke-test branch April 10, 2023 17: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.

3 participants