search

derekkraan / horde

Horde is a distributed Supervisor and Registry backed by DeltaCrdt
MIT License
1.32k stars 106 forks source link

Rename Horde.Supervisor to Horde.DynamicSupervisor #131

Closed derekkraan closed 5 years ago

derekkraan commented 5 years ago

Now that we are really API-compatible with Elixir.DynamicSupervisor, to avoid confusion in the future we should rename Horde.Supervisor to Horde.DynamicSupervisor.

treble37 commented 5 years ago

Just starting to go through horde for the first time and was starting to rename...

So I see lib/horde/horde_dynamic_supervisor.ex which already contains Horde.DynamicSupervisor...so did you want to rename Horde.Supervisor to something else or am I misunderstanding this issue?

derekkraan commented 5 years ago

Horde.DynamicSupervisor is currently a private module in Horde, you should not use it directly. I would like to rename what is now called Horde.Supervisor to Horde.DynamicSupervisor, and the current module named that will have to be called something else.

So this is an issue describing what will happen, it is not already like that.

shikanime commented 5 years ago

@derekkraan As i understand so far, since Horde.DynamicSupervisor is the supervisor which maintains local processes we could rename it as WorkersSupervisor which may be more meaningful of his responsibility.

derekkraan commented 5 years ago

@Shikanime it is really just a distributed version of Elixir.DynamicSupervisor.

shikanime commented 5 years ago

@derekkraan So can we use its API directly instead of using a forked elixir module, or what is the reason that the Horde is using this module? 🤔

derekkraan commented 5 years ago

Right now there is a (private) module in Horde called Horde.DynamicSupervisor, this module is a modified version of Elixir.DynamicSupervisor. I would like to rename this to something else, so that we can rename the module currently called Horde.Supervisor to Horde.DynamicSupervisor.

Horde.DynamicSupervisor -> Horde.SomethingElse Horde.Supervisor -> Horde.DynamicSupervisor

The reason being that Horde.Supervisor mirrors the API of Elixir.DynamicSupervisor, not Elixir.Supervisor.

Is this making sense now? I realize that it's a bit confusing since there is already a module called Horde.DynamicSupervisor.

derekkraan commented 5 years ago

Done in d47a068