search

theforeman / foreman_ansible_inventory

Foreman dynamic inventory script for ansible - Now merged into Ansible itself
GNU General Public License v3.0
70 stars 31 forks source link

Prefer hostgroup title over hostgroup name #39

Closed agx closed 7 years ago

agx commented 7 years ago

Foreman changed behviour in 1.12: the hostgroup_name does no longer contain the parent hostgroups like " a / b / c " but the title does so use this for the building the ansible groups. Fall back to hostgroup_name for older versions.

Closes #27

smnmtzgr commented 7 years ago

I think your change wouldn`t be functional as the "group"-variable is missing and so line 262 will fail.

agx commented 7 years ago

On Wed, Oct 12, 2016 at 11:48:43PM -0700, Simon wrote:

I think your change wouldn`t be functional as the "group"-variable is missing and so line 262 will fail.

You're right. No idea why this didn't trigger in my tests. I've updated the PR but I'm fine with yours if you unbreak older Foreman by handling _name as well (See 260 in my PR).

You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/theforeman/foreman_ansible_inventory/pull/39#issuecomment-253428644

smnmtzgr commented 7 years ago

done, please check again.

agx commented 7 years ago

On Thu, Oct 13, 2016 at 04:54:45AM -0700, Simon wrote:

done, please check again.

Looks good. Can you squash the 4 commits into one with a nice commit message so we get a feature based histoy instead of a time based one?

I'm happy to merge it then.

smnmtzgr commented 7 years ago

squashed into one commit. Please check again.

agx commented 7 years ago

Not needed due to #38