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

make the copper app buildable from a CopperContext #185

Merged
merged 6 commits into from
Dec 30, 2024

Conversation

hscoelho
Copy link
Contributor

This is the draft PR to discuss the changes in the issue #135

I have made commits to add both the builder pattern and a "new_with_context" method. I did both because I'm not sure which one is actually better, I like the ergonomics of the builder pattern(and it was nice to write), but I like the simplicity of new_with_context 😅. When it's decided which one it's better, I will remove the other one.

I also updated the balance bot example to use the builder pattern, to show how it's used both in the sim_mode and non sim_mode.
I can update the other examples when the changes are settled.

this method takes a CopperContext and calls the new function with the context clock and logger
the builder uses the builder pattern and is able to construct an app from the CopperContext or the clock and logger individually
@gbin gbin self-requested a review December 28, 2024 15:19
@gbin
Copy link
Collaborator

gbin commented Dec 28, 2024

I really like the builder pattern, I think this will be pretty intuitive for Rust users. Let's update the other examples and merge the PR!

@hscoelho hscoelho marked this pull request as ready for review December 29, 2024 00:14
@hscoelho
Copy link
Contributor Author

I updated the rest of the examples and removed the new_with_context!

examples/cu_rp_balancebot/src/sim.rs Show resolved Hide resolved
@gbin gbin merged commit 8ba23cc into copper-project:master Dec 30, 2024
3 checks passed
@gbin
Copy link
Collaborator

gbin commented Dec 30, 2024

Thank you @hscoelho !

@hscoelho hscoelho deleted the build-from-context branch January 2, 2025 13:47
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.

2 participants