Add GenServer.format_status/1 callback #13511
Conversation
lib/elixir/lib/gen_server.ex
Outdated
| @doc """ | ||
| This function is called by a `GenServer` process in the following situations: |
There was a problem hiding this comment.
I copied the erlang docs and tweaked them for Elixir
| end | ||
|
|
||
| def format_status(_, [_pdict, %{mod: mod} = state]) do | ||
| [data: [{~c"State", state}], supervisor: [{~c"Callback", mod}]] |
There was a problem hiding this comment.
I don't think we can reproduce this behavior with format_status/1, and this breaks a test - should I just keep this one around for now?
There was a problem hiding this comment.
Or we can just remove these tests, or implement them to use :mod?
If :supervisor.get_callback_module is used somewhere which matters, should erlang add a key to the format_status map?
There was a problem hiding this comment.
@sabiwara we should mirror whatever the Erlang supervisor is doing. This was most likely just borrowed from them.
There was a problem hiding this comment.
OK nice, seems they just removed it.
erlang/otp@67513dc#diff-030fc115adbcd287aecbd623b8355c9cf9aa1fd05ef07b4001b05ce927ec5e03
OK to remove the tests completely then?
| @@ -775,6 +807,7 @@ defmodule GenServer do | |||
| list of `{key, value}` tuples representing the current process dictionary of | |||
| the `GenServer` and `state` is the current state of the `GenServer`. | |||
| """ | |||
| @doc deprecated: "Use format_status/1 callback instead" | |||
| @callback format_status(reason, pdict_and_state :: list) :: term | |||
| when reason: :normal | :terminate | |||
There was a problem hiding this comment.
I wonder if we should still document it. In any case, let's also add a TODO to remove it on 2.0?
There was a problem hiding this comment.
I agree, let's remove the docs completely.
There was a problem hiding this comment.
Let's add the TODO and ship it!
#11220
I'm not sure if something else has to be done, opening my progress so far for discussion.
DynamicSupervisor,GenEvent) are defining new keys likesupervisorwhich don't seem possible anymore?DynamicSupervisoris adding asupervisor: [{~c"Callback", ...key, needed for thesupervisormodule. Unfortunately, I don't think there's a way to add something that will end up in theMiscpart withformat_status/1, so will probably need to revert this one toformat_status/2?GenEventhas been deprecated for 6y, maybe it can stay on the deprecated callback?:gen_eventitself seems to be defining its ownformat_status/1(source), but our docs encourage to use:gen_eventdirectly so might be OK?