nickjj/ansigenome

86a62d40adac956784510ec11fe8ee197a9f05c8 broke `role.galaxy_name`

Closed this issue · 4 comments

ypid commented

I already mentioned this in #47 (comment)
86a62d4 broke role.galaxy_name and with this also the links based on it.

I did not look more closely what the patch actually does.

Here is a mini working example using this template:

{{ role.galaxy_name | replace('-', '--')}}
{{ role.name | replace('-', '--')}}

And this role as an example: https://github.com/debops-contrib/ansible-apparmor

With 86a62d4, this generates the following:

debops--contrib.debops--contrib.apparmor
debops--contrib.apparmor

And with eb85483 (one commit before), it generates this:

debops--contrib.apparmor
apparmor

cc: @jimbocoder

PS: Might come in handy, I wrote a little wrapper script for ansigenome gendoc to handle multiple environments: https://github.com/ypid/ypid-ansible-common/tree/master/template_READMEs

I agree that this change should be reversed. It's good to be able to control the naming on the template level, leaving the decision to the role author.

Thanks Robin and @drybjed. Unfortunately straight up reversing the change just introduces a regression to other reported issues, so let's figure out a real fix. I hope to spend some time with this this weekend. In the mean time I'll paste some quick thoughts I had earlier as a request for further comments:

I think it's a bit tricky because "normalizing" the role identifier by stripping the company name out breaks graph generation, but apparently is necessary for doc generation. So even if it were configurable, you're broken one way or the other.

What if we look for an optional key in meta/ansigenome.yml called display_name? If present, ansigenome will use that when rendering the role's simple name, otherwise it will use the role's complete name.

ypid commented

@jimbocoder Thanks for working on this! I think role.galaxy_name and role.name should be sufficient. If not you can make this configurable in ansigenome.conf and maybe optionally also in meta/ansigenome.yml. Looking forward to your patch to make this work for all of us.

Sorry, was traveling for a bit so I missed this issue. I will merge in the proper fix when it becomes available.