Skip to content
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

Fix image builder #3544

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

maximiliankolb
Copy link
Contributor

What changes are you introducing?

hide procedures for upstream that are only relevent for RHEL7/6 + some minor cleanup

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

?

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

please review commit by commit; I suggest to squash the commits before merging unless someone comments otherwise.

Checklists

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

  • Foreman 3.13/Katello 4.15
  • Foreman 3.12/Katello 4.14 (Satellite 6.16)
  • Foreman 3.11/Katello 4.13 (orcharhino 6.11 on EL8 only; orcharhino 7.0 on EL8+EL9)
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • We do not accept PRs for Foreman older than 3.7.

Copy link

github-actions bot commented Jan 2, 2025

The PR preview for 58c5923 is available at theforeman-foreman-documentation-preview-pr-3544.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

@@ -65,8 +65,3 @@ This creates a new `rhel6-cloud.qcow2` file in the location where you enter the
====
You must manually resize the partitions of instances based on the image in accordance with the disk space in the flavor that is applied to the instance.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider this out of scope unless someone provides a suggestion.

Copy link
Contributor

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for removing the duplicate next steps!

Generally looks good to me. My comment isn't blocking your PR.

@@ -10,8 +10,10 @@ include::modules/proc_registering-a-host.adoc[leveloffset=+1]

include::modules/proc_installing-and-configuring-puppet-agent-manually.adoc[leveloffset=+1]

ifdef::satellite[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. Aren't they still relevant for upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically yes, upstream users could still use RHEL 6 (???) or RHEL 7/Oracle Linux 7 with paid support. Though I'd argue that it's OK to drop this by now.

line 13 was basically a judgement call by me. I am happy to adjust this to "if not orcharhino".

@maximiliankolb maximiliankolb added style review done No issues from docs style/grammar perspective Needs tech review Requires a review from the technical perspective labels Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs tech review Requires a review from the technical perspective style review done No issues from docs style/grammar perspective
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants