m4rcu5nl / ansible-role-zerotier

Ansible role to install Zerotier-One and join a Zerotier network
GNU General Public License v3.0
77 stars 50 forks source link

Ensure curl is installed. #13

Closed flantel closed 6 years ago

flantel commented 6 years ago

Not always installed by default. Install will fail without it.

andyshinn commented 6 years ago

If this is going to be in role I would suggest making it an optional step (controlled with maybe zerotier_install_curl) and setting to false by default. It isn't something I would want installed by a role and I already handle it elsewhere. Thoughts?

Edit: I've got a branch with a bunch of fixes for this role at https://github.com/andyshinn/ansible-role-zerotier/commit/f1cc5c4e497f40c73fb5c32ab964a6661a3a5337. One of them is to use the uri module so curl isn't needed. Do you think this makes sense to open as a PR?

flantel commented 6 years ago

I don't see an issue with ensuring curl is installed given that it is a dependency.

URL module might be a good idea

m4rcu5nl commented 6 years ago

Sorry for taking so long to reply to this PR. My initial thought was much in line with @andyshinn. I aim to keep the roles as clean as possible. Basic dependencies like Curl should be handled by a separate common/kickstart role or even in the playbook evoking this role imo.

@andyshinn Eliminating the need for Curl entirely sounds like a good approach. I'll take a look at your other fixes in the following days and get back to you.

m4rcu5nl commented 6 years ago

This improvement is no longer needed. #15 eliminates the need for Curl in the role altogether.