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

Removed Main; changed weaver.Run to take lambda. #409

Merged
merged 1 commit into from
Jun 21, 2023
Merged

Conversation

mwhittaker
Copy link
Member

This PR reverts #359. In particular, it removes the main component's Main method and changes weaver.Run to take a lambda.

We made this change for the following reason. The Main method is run by weaver.Run, but not by weavertest.Runner.Test. We felt this was confusing. With the lambda API, it is much clearer what is run. This change also unifies the weaver.Run and weavertest.Runner.Test API. Both take a lambda with component arguments.

Right now, the signature of weaver.Run is a bit gnarly. I think I can simplify it a bit in a future PR to remove PointerToMain at least.

@mwhittaker mwhittaker requested a review from ghemawat June 20, 2023 21:00
@mwhittaker mwhittaker self-assigned this Jun 20, 2023
Copy link
Collaborator

@ghemawat ghemawat left a comment

Choose a reason for hiding this comment

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

Thanks.

Base automatically changed from test_impls to main June 21, 2023 18:31
This PR reverts #359. In particular, it removes the main component's
Main method and changes `weaver.Run` to take a lambda.

We made this change for the following reason. The Main method is run by
`weaver.Run`, but not by `weavertest.Runner.Test`. We felt this was
confusing. With the lambda API, it is much clearer what is run. This
change also unifies the `weaver.Run` and `weavertest.Runner.Test` API.
Both take a lambda with component arguments.

Right now, the signature of `weaver.Run` is a bit gnarly. I think I can
simplify it a bit in a future PR to remove PointerToMain at least.
@mwhittaker mwhittaker merged commit a4a3f37 into main Jun 21, 2023
7 checks passed
@mwhittaker mwhittaker deleted the run_lambda branch June 21, 2023 18:38
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