-
Notifications
You must be signed in to change notification settings - Fork 107
Rework docs for Alternate Content Sources (ACS) #4492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rework docs for Alternate Content Sources (ACS) #4492
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReworks the Alternate Content Sources (ACS) documentation by splitting procedures into Web UI vs CLI modules, simplifying anchors and filenames, and restructuring content to facilitate future addition of Deb content workflows. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
07dcdf8 to
411e97f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Since you renamed/split several ACS modules, consider keeping the old anchors or adding alias/xref targets so any existing internal or external links to the removed modules continue to resolve correctly.
- With separate Web UI and CLI procedures that are very similar, it might be worth extracting common prerequisites or conceptual explanations into shared modules to reduce duplication and keep future updates easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since you renamed/split several ACS modules, consider keeping the old anchors or adding alias/xref targets so any existing internal or external links to the removed modules continue to resolve correctly.
- With separate Web UI and CLI procedures that are very similar, it might be worth extracting common prerequisites or conceptual explanations into shared modules to reduce duplication and keep future updates easier.
## Individual Comments
### Comment 1
<location> `guides/common/modules/proc_creating-rhui-alternate-content-sources-by-using-web-ui.adoc:39` </location>
<code_context>
+. If SSL verification is required, enable *Verify SSL* and select the SSL CA certificate.
+. Click *Add*.
+. Navigate to *Content* > *Alternate Content Sources*.
+. Click the vertical ellipsis next to the newly created alternate content source and click *Refresh*.
--- /dev/null
+++ b/guides/common/modules/proc_creating-custom-alternate-content-sources-by-using-web-ui.adoc
</code_context>
<issue_to_address>
**issue (review_instructions):** This Web UI procedure ends after the refresh step without telling users how to verify that the RHUI alternate content source refresh completed successfully.
Please add a brief Verification section (for example, pointing users to Monitor > Tasks or another suitable check) so they can confirm that the ACS creation and refresh completed without errors, consistent with the other ACS procedures that already provide verification steps.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `guides/common/modules/proc*.adoc`
**Instructions:**
Verification steps are included where appropriate.
</details>
</issue_to_address>
### Comment 2
<location> `guides/common/modules/proc_synchronizing-smart-proxy-directly-from-red-hat-cdn-by-using-web-ui.adoc:22` </location>
<code_context>
+. Review details and click *Add*.
+. Navigate to *Content* > *Alternate Content Sources*, click the vertical ellipsis next to the newly created alternate content source, and select *Refresh*.
++
+Your {SmartProxyServer} will now download content from Red{nbsp}Hat CDN and not from {ProjectServer}.
--- /dev/null
+++ b/guides/common/modules/ref_permissions-required-to-manage-alternate-content-sources.adoc
</code_context>
<issue_to_address>
**issue (review_instructions):** This synchronization procedure lacks explicit verification steps to confirm that the Smart Proxy is actually downloading content directly from Red Hat CDN after configuration.
Consider adding a Verification section that tells users how to confirm the new behavior (for example, by checking the relevant tasks, logs, or ACS details) so they can be sure the Smart Proxy is now syncing from Red Hat CDN instead of {ProjectServer}.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `guides/common/modules/proc*.adoc`
**Instructions:**
Verification steps are included where appropriate.
</details>
</issue_to_address>
### Comment 3
<location> `guides/common/modules/proc_creating-rhui-alternate-content-sources-by-using-cli.adoc:20` </location>
<code_context>
+----
+# rhui-manager repo info --repo_id _My_Repo_ID_
+----
+* Note that the alternate content source paths consist of a base URL appended with the subpaths that you provide.
+For example, if your base URL is `\https://{server-example-com}` and your subpaths are `rhel7/` and `rhel8/`, then both `\https://{server-example-com}/rhel7/` and `\https://{server-example-com}/rhel8/` will be searched.
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The phrase "are ... searched" in the following example uses passive voice; consider rewriting it in active voice.
In the next line, "then both `https://{server-example-com}/rhel7/` and `https://{server-example-com}/rhel8/` will be searched" uses passive voice.
To align with the style guide, you could rephrase it to something like: "then the system searches both ..." or "then {Project} searches both ..."
<details>
<summary>Review instructions:</summary>
**Path patterns:** `guides/common/*.adoc,guides/common/modules/*.adoc`
**Instructions:**
Avoid passive voice in verbs.
</details>
</issue_to_address>
### Comment 4
<location> `guides/common/modules/proc_creating-custom-alternate-content-sources-by-using-cli.adoc:12` </location>
<code_context>
+----
+# rhui-manager repo info --repo_id _My_Repo_ID_
+----
+* Note that the alternate content source paths consist of a base URL appended with the subpaths that you provide.
+For example, if your base URL is `\https://{server-example-com}` and your subpaths are `rhel7/` and `rhel8/`, then both `\https://{server-example-com}/rhel7/` and `\https://{server-example-com}/rhel8/` will be searched.
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The example sentence that follows this line ends with "will be searched," which is passive voice and should be rewritten actively.
Right after this line, the example says that both URLs "will be searched," which is passive.
You could rephrase it as "then the system searches both ..." or "then {Project} searches both ..." to keep the meaning while avoiding passive voice.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `guides/common/*.adoc,guides/common/modules/*.adoc`
**Instructions:**
Avoid passive voice in verbs.
</details>
</issue_to_address>
### Comment 5
<location> `guides/common/modules/proc_creating-rhui-alternate-content-sources-by-using-web-ui.adoc:20` </location>
<code_context>
+----
+# rhui-manager repo info --repo_id _My_Repo_ID_
+----
+* Note that the alternate content source paths consist of a base URL appended with the subpaths that you provide.
+For example, if your base URL is `\https://{server-example-com}` and your subpaths are `rhel7/` and `rhel8/`, then both `\https://{server-example-com}/rhel7/` and `\https://{server-example-com}/rhel8/` will be searched.
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The subsequent example ends with "will be searched," which is passive voice and should be made active.
The example that follows this line says both URLs "will be searched," which uses passive voice.
Consider changing it to something like "then the system searches both ..." or "then {Project} searches both ..." to comply with the style guidance.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `guides/common/*.adoc,guides/common/modules/*.adoc`
**Instructions:**
Avoid passive voice in verbs.
</details>
</issue_to_address>
### Comment 6
<location> `guides/common/modules/proc_creating-rhui-alternate-content-sources-by-using-web-ui.adoc:31` </location>
<code_context>
+Ensure that you pass the repo labels of the desired repositories.
+. In the *Name* field, enter a name for your RHUI ACS.
+. Optional: In the *Description* field, provide a description for the ACS.
+. Select {SmartProxies} to which the alternate content source is to be synchronized.
+. If you require synchronizing content through the HTTP proxy of your {SmartProxies}, select *Use HTTP proxies*.
+. In the *Base URL* field, enter the base URL of the Red{nbsp}Hat Update Infrastructure CDS node.
</code_context>
<issue_to_address>
**suggestion (review_instructions):** "is to be synchronized" is passive; rephrase to use active voice.
To avoid passive voice, you could rewrite this as something like:
- "Select {SmartProxies} that you want to synchronize with this alternate content source."
This keeps the user action clear and uses active voice.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `guides/common/*.adoc,guides/common/modules/*.adoc`
**Instructions:**
Avoid passive voice in verbs.
</details>
</issue_to_address>
### Comment 7
<location> `guides/common/modules/proc_creating-custom-alternate-content-sources-by-using-web-ui.adoc:22` </location>
<code_context>
+Ensure that you pass the repo labels of the desired repositories.
+. In the *Name* field, enter a name for your RHUI ACS.
+. Optional: In the *Description* field, provide a description for the ACS.
+. Select {SmartProxies} to which the alternate content source is to be synchronized.
+. If you require synchronizing content through the HTTP proxy of your {SmartProxies}, select *Use HTTP proxies*.
+. In the *Base URL* field, enter the base URL of the Red{nbsp}Hat Update Infrastructure CDS node.
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The construction "is to be synchronized" is passive and should be rewritten in active voice.
You could rephrase this line as, for example:
- "Select {SmartProxies} that you want to synchronize with this alternate content source."
This makes the sentence active and more user-action oriented.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `guides/common/*.adoc,guides/common/modules/*.adoc`
**Instructions:**
Avoid passive voice in verbs.
</details>
</issue_to_address>
### Comment 8
<location> `guides/common/modules/proc_creating-simplified-alternate-content-sources-by-using-web-ui.adoc:16` </location>
<code_context>
+Ensure that you pass the repo labels of the desired repositories.
+. In the *Name* field, enter a name for your RHUI ACS.
+. Optional: In the *Description* field, provide a description for the ACS.
+. Select {SmartProxies} to which the alternate content source is to be synchronized.
+. If you require synchronizing content through the HTTP proxy of your {SmartProxies}, select *Use HTTP proxies*.
+. In the *Base URL* field, enter the base URL of the Red{nbsp}Hat Update Infrastructure CDS node.
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The phrase "is to be synchronized" uses passive voice; consider an active formulation.
To avoid passive voice, you could change this to something like:
- "Select {SmartProxies} that you want to synchronize with this alternate content source."
This keeps the focus on the user's action and adheres to the style guidance.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `guides/common/*.adoc,guides/common/modules/*.adoc`
**Instructions:**
Avoid passive voice in verbs.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
guides/common/modules/proc_creating-custom-alternate-content-sources-by-using-cli.adoc
Outdated
Show resolved
Hide resolved
guides/common/modules/proc_creating-rhui-alternate-content-sources-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
guides/common/modules/proc_creating-custom-alternate-content-sources-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
guides/common/modules/proc_creating-simplified-alternate-content-sources-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
Lennonka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Suggestions below.
...common/modules/proc_synchronizing-smart-proxy-directly-from-red-hat-cdn-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
guides/common/modules/proc_creating-custom-alternate-content-sources-by-using-cli.adoc
Outdated
Show resolved
Hide resolved
guides/common/modules/proc_creating-rhui-alternate-content-sources-by-using-cli.adoc
Outdated
Show resolved
Hide resolved
guides/common/modules/proc_creating-rhui-alternate-content-sources-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
...common/modules/proc_synchronizing-smart-proxy-directly-from-red-hat-cdn-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
...common/modules/proc_synchronizing-smart-proxy-directly-from-red-hat-cdn-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
maximiliankolb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I applied all suggestions by Lena. Kindly asking for a re-review.
guides/common/modules/proc_creating-custom-alternate-content-sources-by-using-cli.adoc
Outdated
Show resolved
Hide resolved
...common/modules/proc_synchronizing-smart-proxy-directly-from-red-hat-cdn-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
...common/modules/proc_synchronizing-smart-proxy-directly-from-red-hat-cdn-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
...common/modules/proc_synchronizing-smart-proxy-directly-from-red-hat-cdn-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
guides/common/modules/proc_creating-rhui-alternate-content-sources-by-using-cli.adoc
Outdated
Show resolved
Hide resolved
Lennonka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️ DITA Vale check is clean
Would it be worth considering incorporating those Sourcery suggestions?
maximiliankolb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into the suggestions by Sourcery and reworded modules as suggested.
guides/common/modules/proc_creating-custom-alternate-content-sources-by-using-cli.adoc
Outdated
Show resolved
Hide resolved
guides/common/modules/proc_creating-simplified-alternate-content-sources-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
guides/common/modules/proc_creating-rhui-alternate-content-sources-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
guides/common/modules/proc_creating-custom-alternate-content-sources-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
Lennonka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few nitpicks. The rest looks good.
guides/common/modules/proc_creating-custom-alternate-content-sources-by-using-web-ui.adoc
Outdated
Show resolved
Hide resolved
Checked all anchors locally: $ rg -i "Managing_Alternate_Content_Sources_" $ rg -i "Configuring_Custom_Alternate_Content_Sources_" $ rg -i "Configuring_RHUI_Alternate_Content_Sources_" $ rg -i "Configuring_Simplified_Alternate_Content_Sources_" $ rg -i "_Directly_From_Red_Hat_CDN_"
Verified removed anchors for CLI procedures manually: $ rg -i "cli-configuring-custom-alternate-content-sources_" $ rg -i "cli-configuring-rhui-alternate-content-sources_" $ rg -i "cli-configuring-simplified-alternate-content-sources_" $ rg -i "directory-from-red-hat-cdn"
This is losely based on "guides/common/modules/ref_permissions-required-to-provision-hosts.adoc".
69c7e1e to
70c36e1
Compare
|
Rebased to HEAD of "master". Still in need of a tiny tech ACK from the Katello Team. Now pinging the person with the second most commits regarding ACS: @ianballou This is the only section that could use a tech ACK: #4492 (comment) |
ianballou
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technical ACK, thanks.
* Simplify anchors for ACS modules Checked all anchors locally: $ rg -i "Managing_Alternate_Content_Sources_" $ rg -i "Configuring_Custom_Alternate_Content_Sources_" $ rg -i "Configuring_RHUI_Alternate_Content_Sources_" $ rg -i "Configuring_Simplified_Alternate_Content_Sources_" $ rg -i "_Directly_From_Red_Hat_CDN_" * Split ACS modules by interface Verified removed anchors for CLI procedures manually: $ rg -i "cli-configuring-custom-alternate-content-sources_" $ rg -i "cli-configuring-rhui-alternate-content-sources_" $ rg -i "cli-configuring-simplified-alternate-content-sources_" $ rg -i "directory-from-red-hat-cdn" * Reword ACS modules * Move ACS permissions into separate module This is losely based on "guides/common/modules/ref_permissions-required-to-provision-hosts.adoc". * Fix description of ACS content flow for Smart Proxy Servers (cherry picked from commit fb72877)
* Simplify anchors for ACS modules Checked all anchors locally: $ rg -i "Managing_Alternate_Content_Sources_" $ rg -i "Configuring_Custom_Alternate_Content_Sources_" $ rg -i "Configuring_RHUI_Alternate_Content_Sources_" $ rg -i "Configuring_Simplified_Alternate_Content_Sources_" $ rg -i "_Directly_From_Red_Hat_CDN_" * Split ACS modules by interface Verified removed anchors for CLI procedures manually: $ rg -i "cli-configuring-custom-alternate-content-sources_" $ rg -i "cli-configuring-rhui-alternate-content-sources_" $ rg -i "cli-configuring-simplified-alternate-content-sources_" $ rg -i "directory-from-red-hat-cdn" * Reword ACS modules * Move ACS permissions into separate module This is losely based on "guides/common/modules/ref_permissions-required-to-provision-hosts.adoc". * Fix description of ACS content flow for Smart Proxy Servers (cherry picked from commit fb72877)
What changes are you introducing?
Why are you introducing these changes? (Explanation, links to references, issues, etc.)
preparation to add docs for Katello/katello#11567
Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)
Contributor checklists
Please cherry-pick my commits into:
Summary by Sourcery
Reorganize and expand Alternate Content Sources documentation to better distinguish workflows by user interface and content type.
Documentation:
Summary by Sourcery
Reorganize Alternate Content Sources documentation to separate workflows by interface and clarify structure.
Documentation: