Skip to content

Bring back the previous behavior of call_guest_function_by_name #761

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jprendes
Copy link
Contributor

@jprendes jprendes commented Aug 4, 2025

This PR brings back the previous behaviour of call_guest_function_by_name, and exposes the new behavioud behind a different name: run.

Originally I wanted to call the new behaviour call, but that method exists as an alias to call_guest_function_by_name throught Callable trait.
The risk of using the call name is that anyone using the Callable trait would get an (almost) silent change in behaviour (almost because there would be a lint about Callable being unused).

Let me know if anyone prefers a different name.

@jprendes jprendes added the kind/bugfix For PRs that fix bugs label Aug 4, 2025
@jprendes jprendes force-pushed the snapshot2 branch 2 times, most recently from ee49c52 to d99d24b Compare August 4, 2025 14:51
dblnz
dblnz previously approved these changes Aug 4, 2025
Copy link
Contributor

@dblnz dblnz left a comment

Choose a reason for hiding this comment

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

LGTM!

ludfjig
ludfjig previously approved these changes Aug 4, 2025
Copy link
Contributor

@ludfjig ludfjig left a comment

Choose a reason for hiding this comment

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

Looks good to me

jsturtevant
jsturtevant previously approved these changes Aug 4, 2025
@jprendes jprendes dismissed stale reviews from jsturtevant and ludfjig via fc3eb8c August 5, 2025 08:51
@jprendes jprendes force-pushed the snapshot2 branch 2 times, most recently from fc3eb8c to 4026423 Compare August 5, 2025 09:12
dblnz
dblnz previously approved these changes Aug 5, 2025
ludfjig
ludfjig previously approved these changes Aug 5, 2025
Copy link
Contributor

@ludfjig ludfjig left a comment

Choose a reason for hiding this comment

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

LGTM. I guess we decided we don't care about the Callable trait?

@jsturtevant
Copy link
Contributor

LGTM. I guess we decided we don't care about the Callable trait?

It looks like we need to provide a way for this to choose the behavior, or we need to break it/keep current behavior

@jprendes jprendes dismissed stale reviews from ludfjig and dblnz via 01d7a69 August 5, 2025 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bugfix For PRs that fix bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants