travis + vagrant + 0.9.x

jaytaylor / ansible-kafka

Ansible Kafka role

https://galaxy.ansible.com/jaytaylor/kafka/

Other

67 stars 52 forks source link

travis + vagrant + 0.9.x #6

Closed aisch closed 8 years ago

aisch commented 8 years ago

closes #3, closes #4

add vagrant so we can test stuff locally
integrate w/ travis so we can safely merge improvements
add 0.9.x support
allow kafka auto broker id assignment (a 0.9.x feature)
allow 0.8.x -> 0.9.x upgrade

aisch commented 8 years ago

@jaytaylor can you enable ci for this repo here: https://travis-ci.org/jaytaylor/ansible-kafka?

jaytaylor commented 8 years ago

@aisch Thank you for opening this PR! I'm grateful for any help to improve this project!

We can get this merged once the above feedback is taken under consideration and if you'll help me understand a few of the changes (again, as noted above).

Thanks again!

aisch commented 8 years ago

travis seems to finally like 0.8.x. 0.9.x is failing due to #4 so ill take a stab at fixing that and then this should be good to merge.

aisch commented 8 years ago

@jaytaylor all passing on travis now (https://travis-ci.org/jaytaylor/ansible-kafka/builds/108960317). i've updated the list of stuff this pr adds. let me know if you spot anything you'd like changed before merge.

thx again for your work on this role.

jaytaylor commented 8 years ago

Awesome work! Only 2 final requests before merging.

1.) Remove the failed_when: check_java.rc not in (0, 1) line since RC of 1 means failure 2.) Remove the debug task

Then we're good to go, and if you are opposed that's OK too! We can discuss until we are on the same page.

aisch commented 8 years ago

ok:

removed the debug message
replaced failed_when: check_java.rc not in (0, 1) w/ ignore_errors: True (we install java when .rc != 0)

jaytaylor commented 8 years ago

LGTM! Thank you @aisch!!!

aisch commented 8 years ago

@jaytaylor thx for the review(s) and merge!