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 Actor::started(), stopped() fallible #89

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Conversation

strohel
Copy link
Member

@strohel strohel commented Aug 12, 2024

I've long time wanted to do this, other work on actor has finally triggered me.

This should allow removing almost all remaining occurrences of fallible .unwrap() / .expect() in our downstream code.

It is (obviously) semver-breaking change which causes a bit of churn to adapt to, but the fact of adapting itself should be a nice cleanup.

@strohel strohel changed the title Make Actor::started(), stopped() fallible Make Actor::started(), stopped() fallible Aug 12, 2024
Copy link
Contributor

@mbernat mbernat left a comment

Choose a reason for hiding this comment

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

Makes sense, neat.

Copy link
Member

@goodhoko goodhoko left a comment

Choose a reason for hiding this comment

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

LGTM!

Let me rebase https://github.com/tonarino/portal/pull/3746 on top of this. It will get rid of quite some lines of expects.

@strohel strohel force-pushed the started-stopped-fallible branch 2 times, most recently from 8983ddb to 44575fd Compare August 13, 2024 12:43
Base automatically changed from error-reporting-qol to main August 13, 2024 13:18
@strohel strohel merged commit 19fc399 into main Aug 13, 2024
6 checks passed
@strohel strohel deleted the started-stopped-fallible branch August 13, 2024 13:22
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