Conversation
sasa1977
left a comment
sasa1977
left a comment
There was a problem hiding this comment.
Looking good so far. I would advise to create a basic test first, which will make it easier to refactor later. Once the code is settled, we'll need to extend docs, both in readme, as well as ExActor.Operations module.
| But by providing a custom implementation of `server_pid/1`, you can map an identifier | ||
| to a PID by some other means. | ||
| """ | ||
| def server_pid(server_reference) do |
There was a problem hiding this comment.
I wonder if we should make this public. By doing it, we're by default adding extra function to the module's interface, and I'm not sure I like that. Preferably, this would be a defp, though I'm not sure if defoverridable would work then. If it doesn't, then we could make it @doc false so it's not included in the generated docs, and marked as an internal detail.
There was a problem hiding this comment.
Also, I think we shouldn't inject this function for the cases where a global :export option is provided.
| @@ -0,0 +1,22 @@ | |||
| defmodule ExActor.Common do | |||
There was a problem hiding this comment.
Since you're introducing this module, then moving the code of ExActor.Helper.init_generation_state to the __using__ might be nice. That way, we get to keep the common boilerplate in the same place.
| quote do | ||
| GenServer.unquote(server_fun)(unquote_splicing(interface_gen_server_args)) | ||
| end | ||
| GenServer.unquote(server_fun)(unquote_splicing(interface_gen_server_args)) |
There was a problem hiding this comment.
This line and the next one need to be indented once more.
2 participants
This implements #27 , custom dispatch logic by using an overridable
server_pid/1function.I 'think' it works, but I have not yet had time to write tests for it (hopefully tomorrow or the day after that), so that is definitely something that should still happen before this pull request is merged, but I wanted to share this in its current state already at least.
Feel free to give feedback. 😄