search

andrewrothstein / ansible-anaconda

Ansible role for installing Anaconda
MIT License
47 stars 42 forks source link

Mac OS X support and test in Travis build #17

Closed remigius42 closed 6 years ago

remigius42 commented 6 years ago

Dear Andrew

First let me thank your for providing your well-written and helpful Ansible roles under a permissive license.

For setting up a basic environment for data science tasks, I am including your ansible-anaconda role as a dependency. As I intend to support Mac OS X as well, I wanted to test it on Travis too. I am not sure, if this is of any interest to you, but since you are including the Mac OS X binaries and hashes in your role, I figured I would open a pull request based on the Travis configuration I am using for my data_science_base role. I tried to keep the changes to a minimum and to follow your coding style where possible.

As an aside, when including ansible-anaconda, I realized that the version published on Ansible Galaxy is v4.0.0, while the latest release in the GitHub repository is v4.0.1, which includes the updates to Anaconda 5.0.1. Is there a particular reason, why the latest GitHub release is not pushed to Ansible Galaxy yet?

Thank you & best regards Andreas

andrewrothstein commented 6 years ago

Thanks for the contribution! I published the role incorporating this change to the galaxy as v4.0.2.

remigius42 commented 6 years ago

Dear @andrewrothstein , thank you for merging and releasing the change. Unfortunately, I am affraid my changes broke version v4.0.2: There is an open Ansible issue #18417 about the variable resolution when using include_vars and with_first_found. When I was looking at your other roles to mimic your coding style, I was wondering why you used the paths parameter, for example in ansible-bash, because I figured the directory would be resolved automatically, which turned out to be true, alas the parent role's directory is resolved first. I opened the pull request #19 to fix it; local tests referring to my fork with the patch seemed to be working.

Sorry for the inconvenience and best regards Andreas

andrewrothstein commented 6 years ago

I should have caught that. Sorry. It's been resolved in v4.0.3.