search

thefinn93 / ansible-letsencrypt

An ansible role to generate TLS certificates and get them signed by Let's Encrypt
GNU General Public License v3.0
442 stars 122 forks source link

Short fixes, cleanup #8

Closed merqlove closed 8 years ago

merqlove commented 8 years ago

Fixed previous PR's. Tested on Ubuntu trusty. Nginx 1.4.6 Debian part is the same as in original.

thefinn93 commented 8 years ago

Okay, same things I asked about the other PR: letsencrypt_command already has the domain arguments. Why are you adding them again (but only in the Ubuntu one?)

They should be as similar as possible, and only do the things that require differences (which appears to be just the package name of virtualenv)

merqlove commented 8 years ago

@thefinn93, Ok, just reviewed merged commits. I think it will work as is. Just changed installed to present. Also special apt for Debian is required. Ubuntu trusty don't have 'virtualenv' package.

thefinn93 commented 8 years ago

Ah, much better! I'm tempted to merge now, but it appears that Ubuntu 16.04 has the virtualenv package, while it is only available in jessie, strech and sid in Debian. Perhaps the conditional for installing it should be somewhat more elaborate...

merqlove commented 8 years ago

We can compare release number as workaround.

thefinn93 commented 8 years ago

Yeah. Would you like to do so before I merge this?

merqlove commented 8 years ago

Am, i will have time for it only tomorrow...Also if you have 16.04 you can merge, add some code & test faster then me :+1:

thefinn93 commented 8 years ago

I do not Ubuntu installed on any of my servers

merqlove commented 8 years ago

14.04 trusty is current major LTS release. 16.04 will be released at 04.16. So we can stop on trusty for now :) I have tested current yaml on trusty.

On 19 янв. 2016 г., at 21:28, Finn notifications@github.com wrote:

I do not Ubuntu installed on any of my servers

— Reply to this email directly or view it on GitHub https://github.com/thefinn93/ansible-letsencrypt/pull/8#issuecomment-172942702.

thefinn93 commented 8 years ago

The other thing to bear in mind is that Ubuntu 16.04 has a letsencrypt package, as does Debian for sid and stretch, so maybe the install procedure should be pretty different on those distros

merqlove commented 8 years ago

Yeah, last steps may change. Anyway we will need test on those systems if we want to have its support :)

thefinn93 commented 8 years ago

True. I guess I'll accept this and open a bug

thefinn93 commented 8 years ago

Thanks for working on this!

merqlove commented 8 years ago

Cheers :+1: