Ensure attribute links get picked up by ConceptLink#4804
Ensure attribute links get picked up by ConceptLink#4804aneta-petrova wants to merge 1 commit intotheforeman:masterfrom
Conversation
github-actions
Bot
added
Needs tech review
aneta-petrova
removed
Needs tech review
|
@maximiliankolb Can you please take a quick look at this? Would this work for you? See the description for context and the problem I'm trying to solve. |
|
Including 
The consequence of this change would be that we'd have to include |
|
Makes sense to me. But shouldn't this be cherry-picked into all to-be-migrated versions? |
Oh yes, and also applied to all occurrences of all guide attributes... 🫠 I just first want to find out if this solution is acceptable to everyone on the team. |
@aneta-petrova This would break downtream builds for me because I overwrite the attribute with a relative path, not a URL. I am unsure if this is working 100% for upstream too. I hope that there's a more elegant solution that this prefix. IMO this would be a huge effort to patch in upstream docs. However, if this is the only way to make it work, I am confident to be able to revert this at build time in my downstream build: Your goal is to make a GHA fail if there are any xrefs/DocURLs in concept modules anywhere (but under ".Additional resources")? |
Why do you think so? The way I imagined this was to simply drop
Yes. I based this solution on jhradilek/asciidoctor-dita-vale#186 (comment). |
|
"huge" is maybe an exaggeration but I count 112 instances:
No, that should be it. I am just wondering (openly) if there is another solution because to me, it feels very un-elegant. Just to clarify: Are links in concepts under "Additional resources" OK? |
They are. Unless that concept is an assembly introduction, because assembly introduction cannot include Additional resources. |
So was I, which is in part why I opened this PR. I welcome suggestions for alternatives. This is the only one I can think of. |
I was wrong! 🥳 Assembly intros can include additional resources. jhradilek/asciidoctor-dita-vale#186 (comment) This will make addressing the requirements of the rule much easier and less disruptive. |
as in "text within an assembly" or "first concept with leveloffset 0 in an assembly"? |
Both :) But for our team, only the second one is relevant. The first concept with leveloffset 0 in assembly can contain an |
|
I'll record the result of this conversation, along with the proposed solution, in a downstream ticket for further processing. For now, we can close this PR; once we decided on whether and how to implement it, we can reopen it... or raise a new one. |
3 participants
What changes are you introducing?
Moving
https://out of link attributes and into the modules.Why are you introducing these changes? (Explanation, links to references, issues, etc.)
jhradilek/asciidoctor-dita-vale#186
Per asciidoctor-dita-vale rules, links and cross-references are not allowed in concept modules. Right now, we often include links as attributes (such as
{ManagingContentDocURL}) and these don't get picked up by the ConceptLink rule. Moving thehttps://URI from attributes to the text ensures that the links get picked up by Vale.Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)
Contributor checklists
Please cherry-pick my commits into: