Update Deprecated :simple_one_to_one to use DynamicSupervisor in Elixir 1.6

[rockerBOO]

• Use dynamic supervision with DynamicSupervisor.
  https://github.com/elixir-lang/elixir/blob/v1.6/CHANGELOG.md#dynamic-supervisor
  https://hexdocs.pm/elixir/DynamicSupervisor.html#init/1
  https://hexdocs.pm/elixir/DynamicSupervisor.html#module-migrating-from-supervisor-s-simple_one_for_one

• Add ExIRC.start_link!/1

Requires Elixir 1.6

[tchoutri]

@rockerBOO Hi, and thank you for this PR! Does it change anything regarding the public API? Could you include an example that leverages the dynamic supervision?

[rockerBOO]

test "client dies if owner process dies" tests this behavior in test/client_test.exs.

ExIRC.start_client! will attach a client to the :exirc supervisor. This can help to make sure the clients die when the main :exirc supervisor dies. We add restart: :temporary because we are OK with the client dying in this case. This should replicate old behavior.

Supervisor.Spec was also deprecated.

Supervisor.Spec
This module is deprecated. Use the new child specifications outlined in the Supervisor module instead.

The functions in this module are deprecated and they do not work with the module-based child specs introduced in Elixir v1.5. Please see the Supervisor documentation instead.

We were using the :simple_one_for_one supervisor, which was also deprecated. The suggestion is to move to a DynamicSupervisor.

Added

To the public API, to allow access to the new module-based child specs, I added ExIRC.start_link!/1 which allows us to do {ExIRC, []} to start the application. We only had ExIRC.start_link!/0. I am dropping the values in the currently but that replicates current behavior. Maybe we should pass options down?

[rockerBOO]

The actual problem case Im having, which might require expanding this (or another pull).

In my tests I want to make a named client that is start_supervised.

  test "connection to the Twitch server" do
    client = start_supervised!(ExIRC)

    Process.register(client, :twitchirc)
  end

This fails because no ExIRC.start_link/1 since start_supervised!(ExIRC) will default to start_supervised!({ExIRC, []})

What I want

What I want to do is like the following. Pass a name opts argument to the client.

  test "connection to the Twitch server" do
    client = start_supervised!({ExIRC, [], name: :twitchirc})
  end

That would mean something like

defmodule ExIRC do 
    @spec start_link!(opts \\ [], process_opts \\ []) 

[tchoutri]

@rockerBOO You can make that addition to this PR. Also, make sure to rebase your branch on the current master.

[rockerBOO]

This allows us to define the module differently if it's started using the dynamic supervisor. There may be a few ways to do this but this seemed the most clear currently.

[rockerBOO]

So after doing some testing I learned that I just needed {ExIRC.Client, []} or ExIRC.Client.start_link/1. We also already have ExIRC.Client.start_link/2 so I can already pass process arg to this. So I removed ExIRC.start_link!/1.

So the key points of the pull request is now only with dynamic supervision to replace the deprecated code.

Let me know if there is something I messed up. The .yaml file seems weird to be adding for example.

[tchoutri]

Beware, you're using your own repo's status for this badge

[rockerBOO]

Good spot :) thanks.

[coveralls]

Coverage increased (+0.3%) to 34.557% when pulling 367fb0a on rockerBOO:new-supervisor into 9ab15b4 on bitwalker:master.

[tchoutri]

@rockerBOO I don't see anything to add. I'll let you aggregate your commits if this PR is ready to be merged. It's a fine work altogether, thank you very much for your patience and efforts.

[rockerBOO]

OK Travis, Github green. Should be good now.

We likely want to increment the number as the version requirement has changed and will cause breaking due to DynamicSupervisor added in 1.6.

Thanks Théophile! Thank you for your help :)

[tchoutri]

Ah, it would seem that Coveralls is unhappy. Do you think you can make it smile again?

And yes, this would warrant a new release.

[rockerBOO]

Ok coveralls is happy.

[tchoutri]

Great, merging it now.
Thank you again @rockerBOO. Glad to have your contribution in the codebase!
If by any chance you were to make an IRC bot made with ExIRC, you can always submit a PR for it to be added in the README.