Skip to content

Conversation

@Ultimate-etamitlU
Copy link
Contributor

Fixing the issue #66031

Version(s):

Issue:

Link to docs preview:

QE review:

  • QE has approved this change.

Additional information:

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 10, 2023
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Oct 10, 2023

🤖 Updated build preview is available at:
https://66032--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/27750

@kalexand-rh kalexand-rh requested a review from skopacz1 October 10, 2023 21:38
@skopacz1
Copy link
Contributor

@bmanzari could you please take a look at this PR and let us know what you think? Thanks!

@bmanzari
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 12, 2023
Copy link
Contributor

@skopacz1 skopacz1 left a comment

Choose a reason for hiding this comment

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

LGTM from a peer review perspective

@skopacz1
Copy link
Contributor

/label peer-review-done
/label merge-review-needed

@openshift-ci openshift-ci bot added peer-review-done Signifies that the peer review team has reviewed this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Oct 12, 2023
@skopacz1
Copy link
Contributor

@Ultimate-etamitlU thanks for your contribution to our docs!

@jab-rh
Copy link
Contributor

jab-rh commented Oct 12, 2023

@skopacz1, do we know what versions this applies to?

@skopacz1
Copy link
Contributor

@jab-rh I'm imagining to all versions where this module exists, so 4.13+, but let me confirm that with an SME/QE. In any case, this PR now needs a rebase, so feel free to remove the merge review label if you'd like me to get everything sorted before requesting another merge review.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 12, 2023
@skopacz1
Copy link
Contributor

skopacz1 commented Oct 12, 2023

Sorry @Ultimate-etamitlU but would you mind rebasing this PR to get rid of the conflicts? I think one of my PRs just merged and caused this issue. (Also, please make sure that you squash any additional commits so that this PR only has one commit at the time of merging)

@skopacz1
Copy link
Contributor

Just confirmed with QE that this can be applied to 4.13+

@jab-rh jab-rh added this to the Continuous Release milestone Oct 12, 2023
@kalexand-rh kalexand-rh removed the merge-review-needed Signifies that the merge review team needs to review this PR label Oct 12, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 12, 2023
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 12, 2023
@openshift-ci
Copy link

openshift-ci bot commented Oct 12, 2023

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 12, 2023
Comment on lines 173 to 193

ifdef::pxe-boot[]

. Optional: To create an iPXE script, add the `bootArtifactsBaseURL` to the `agent-config.yaml` file:
+
[source,yaml]
----
apiVersion: v1beta1
kind: AgentConfig
metadata:
name: sno-cluster
rendezvousIP: 192.168.111.80
bootArtifactsBaseURL: <asset_server_URL>
----
+
Where `<asset_server_URL>` is the URL of the server you will upload the PXE assets to.
endif::pxe-boot[]

ifeval::["{context}" == "prepare-pxe-assets-agent"]
:!pxe-boot:
endif::[] No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

This is content that I merged earlier, which triggered the conflict, but this should be included in the main branch of the docs. Can you add this content back in, and then squash all of your commits into one?

Trying to merge all in one
@openshift-ci openshift-ci bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 12, 2023
Copy link
Contributor Author

@Ultimate-etamitlU Ultimate-etamitlU left a comment

Choose a reason for hiding this comment

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

Merging all in one

@Ultimate-etamitlU
Copy link
Contributor Author

Tried by GUI, don't have CLI so will be adding up it tomorrow. Not fond of CLI that much so may be missing something over GUI to collect everything at a place.

@Ultimate-etamitlU Ultimate-etamitlU closed this by deleting the head repository Oct 12, 2023
@Ultimate-etamitlU
Copy link
Contributor Author

Closing this one as there are multiple changes I have on a single file and would be adding a fresh PR to avoid confusion here.
Sorry for little noise but rebasing from GUI or dealing with PR's raise from GUI and dealing with those with CLI is expert task :/

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

Labels

branch/enterprise-4.13 branch/enterprise-4.14 peer-review-done Signifies that the peer review team has reviewed this PR 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.

7 participants