Skip to content

Update README.md #197

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

Merged
merged 2 commits into from
Jan 13, 2025
Merged

Update README.md #197

merged 2 commits into from
Jan 13, 2025

Conversation

FranzBusch
Copy link
Contributor

@FranzBusch FranzBusch commented Jan 12, 2025

The readme shows an actor implementing the Service protocol. This has lead to some people thinking that only actors can do this so let's switch this to a class which is the usual type conforming to Service.

@FranzBusch FranzBusch added the semver/none No version bump required. label Jan 12, 2025
@FranzBusch FranzBusch enabled auto-merge (squash) January 12, 2025 16:31
@czechboy0
Copy link
Collaborator

Why not a struct? Most services I conform are actually either structs or actors, but rarely classes, as those then require manual locking.

@gjcairo
Copy link
Collaborator

gjcairo commented Jan 13, 2025

Yeah I kind of agree that class may also be misleading.

Regardless of whether we keep class or change it to struct though, can we not add a comment saying that it actually doesn't matter whether the type is a class, an actor or a struct? If it's brought confusion in the past, let's make it super obvious.

@FranzBusch
Copy link
Contributor Author

@czechboy0 @gjcairo updated the example

@FranzBusch FranzBusch merged commit c641ece into main Jan 13, 2025
22 checks passed
@FranzBusch FranzBusch deleted the fb-actor-to-class branch January 13, 2025 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/none No version bump required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants