-
Notifications
You must be signed in to change notification settings - Fork 54
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
Improve naming for message box builder traits #1748
Improve naming for message box builder traits #1748
Conversation
acd07a7
to
9adcbd9
Compare
9adcbd9
to
d588c51
Compare
Robot Results
Passed Tests
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not the biggest fan of the name Server
as that still sounds a bit too broad. But, still approving as I'm unable to come up with something better.
@@ -341,22 +341,22 @@ impl<Request: Message, Response: Message> MessageBox | |||
/// Client side handler of requests/responses sent to an actor | |||
/// | |||
/// Note that this message box sends requests and receive responses. | |||
pub struct RequestResponseHandler<Request, Response> { | |||
pub struct ServerClientMessageBox<Request, Response> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just ClientMessageBox
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, an option I considered. I tried to keep the Server prefix. But, you are correct, ClientMessageBox
is simpler => I will do the change before merging.
d588c51
to
0ed99ef
Compare
- The terms Socket and Plug failed to convey the intent of these two traits. - ServiceProvider stresses that an actor, which builder implements that trait, provides a service that can be used by other actors. - ServiceConsumer is the trait to be implemented by the builder on an actor that use a service provider by an other. Alternatives that have been considered: - FeatureProvider and FeatureConsumer - Adding a Builder suffix as these traits are to be implemented by builders - Adding a Connector suffix as these traits are used to connect actors. Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
This is to avoid confusion with ServiceProvider and ServiceConsumer that are implemented by actors that don't act as servers i.e. with a protocol that is not necessarily a request/response protocol. Renaming: - ServiceMessageBox -> ServerMessageBox - ServiceMessageBoxBuilder -> ServerMessageBoxBuilder - RequestResponseHandler -> ClientMessageBox Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
To avoid confusion with ServiceConsumer and ServiceProvider. Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
0ed99ef
to
85a4457
Compare
Proposed changes
Types of changes
Paste Link to the issue
#1724
Checklist
cargo fmt
as mentioned in CODING_GUIDELINEScargo clippy
as mentioned in CODING_GUIDELINES