Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Oct 8, 2020

xrefs aren't allowed in modules:

$ grep -A1 'You must not include xrefs' modules/mod-docs-ocp-conventions.adoc
You must not include xrefs in modules or create an xref to a module. You can
only use xrefs to link from one assembly to another.

But this module only has a single consumer:

$ git --no-pager grep 'include.*virt-about-upgrading-virt' origin/master
origin/master:virt/upgrading-virt.adoc:include::modules/virt-about-upgrading-virt.adoc[leveloffset=+1]

So shift it into the virt/ assembly to allow the xref added in 87623ba (#24172). CC @sjhala-ccs

xrefs aren't allowed in modules:

  $ grep -A1 'You must not include xrefs' modules/mod-docs-ocp-conventions.adoc
  You must not include xrefs in modules or create an xref to a module. You can
  only use xrefs to link from one assembly to another.

But this module only has a single consumer:

  $ git --no-pager grep 'include.*virt-about-upgrading-virt' origin/master
  origin/master:virt/upgrading-virt.adoc:include::modules/virt-about-upgrading-virt.adoc[leveloffset=+1]

So shift it into the virt/ assembly to allow the xref added in
87623ba (BZ-1857788 Added note about upgrade workaround for VMs
using HPP, 2020-07-24, openshift#24172).
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 8, 2020
@bergerhoffer
Copy link
Contributor

While I understand the desire to make this change to allow the xrefs, we can't just move/rename a module to make it an assembly.

@bergerhoffer
Copy link
Contributor

The best way to move forward with this would be to remove the xref from the module. And if it's an important enough link, it could be included in the assembly, in the "Additional resources" section.

@sjhala-ccs
Copy link
Contributor

@bergerhoffer I agree. @wking I can take care of removing the xref from the module and moving the link to the assembly.

@wking
Copy link
Member Author

wking commented Oct 8, 2020

While I understand the desire to make this change to allow the xrefs, we can't just move/rename a module to make it an assembly.

Why not? Because that's what I've been doing in #26219 for docs that no other assembly will need to include, and I don't want to go too far down something that turns out to be a dead-end ;)

@bergerhoffer
Copy link
Contributor

Why not? Because that's what I've been doing in #26219 for docs that no other assembly will need to include, and I don't want to go too far down something that turns out to be a dead-end ;)

I can chat with you in more detail if you want to learn more about our modular docs structure, but basically the way to look at it is that the majority of content should be separated into module. Each module could be conceptual, or procedural, or reference info. And then you look at an assembly as the glue that holds these modules together to provide a single user story. There is some information contained in an assembly, but it typically should be mostly introductory information, additional information, etc. There are exceptions of course, but that's the typical view to take.

Modules and assemblies also have differences in the metadata and other conventions. If you want to learn more about the structure, this is our mod docs reference manual: https://redhat-documentation.github.io/modular-docs/

Also, even if you don't expect something to need to be reused, information should still be broken up into modules and glued together by an assembly. Looking at the PR that you're referencing, you do currently have too much information in the assemblies that should be broken out into modules. And typically, assemblies don't include other assemblies, they only include other modules. I don't have the background on this document you're adding, but I'd be happy to help you figure out how it should be restructured.

@wking
Copy link
Member Author

wking commented Oct 8, 2020

Looking at the PR that you're referencing, you do currently have too much information in the assemblies that should be broken out into modules.

This isn't a cost-benefit between "reusable between different assemblies" (favoring modules) and "allows xref" (favoring assembly-dir files)? I feel like on-location xrefs are useful enough that it is surprising if we can make a blanket ruling in favor of modules.

@bergerhoffer
Copy link
Contributor

This isn't a cost-benefit between "reusable between different assemblies" (favoring modules) and "allows xref" (favoring assembly-dir files)? I feel like on-location xrefs are useful enough that it is surprising if we can make a blanket ruling in favor of modules.

This is wider than just the OpenShift docs. The entire CCS organization follows this modular docs approach. This is important for consistency across our product documentation.

There really shouldn't be an "assembly-dir file" as you call it. The guide I pointed you to has a definition of what an assembly is here [1]. Just because you want to xref in this case doesn't mean that you should put the entire thing in an assembly. We have other workarounds, like I mentioned earlier. The xrefs can be added as links in the assembly intro, or prereqs, or additional resources section.

As much as we might like to xref in modules too, we have to adhere to the current "no xrefs in modules" guideline, which should hopefully be lifted at some point. If you'd like to contribute docs to our repo, we'd love that. But you do need to follow our guidelines in order for it to be merged in.

[1] https://redhat-documentation.github.io/modular-docs/#assembly-definition

@vikram-redhat
Copy link
Contributor

+1 to @bergerhoffer's last comment. I don't think we want to restructure the type of a file (from an assembly to a module), just because it doesn't fit the narrative in the context. While this module might only be getting used once, for now, it defeats the purpose of making it a module in the long run if we allow an xref, or change its type to an assembly.

@wking I am gonna close this PR. @sjhala-ccs has created the PR (#26268) to remove the xref and move the link to an assembly, that was added erroneously in #24172.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants