86a62d40adac956784510ec11fe8ee197a9f05c8 broke `role.galaxy_name`
Closed this issue · 4 comments
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
calleddisplay_name
? If present, ansigenome will use that when rendering the role's simple name, otherwise it will use the role's complete name.
@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.