-
Couldn't load subscription status.
- Fork 1.8k
OADP-5888 Add Line After Include Statements #93812
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
Conversation
8a1cf5b to
3db2bd3
Compare
|
/label peer-review-needed |
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.
LGTM
|
|
||
| [id="oadp-different-kubernetes-api-versions"] | ||
| == Working with different Kubernetes API versions on the same cluster | ||
|
|
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.
This is out of the scope of this PR, but this section needs a short introduction. Otherwise, you end up with one heading being displayed just after another heading.
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.
This is also part of CQA 2.0 work for migration and will need to be done at some point as I understand.
| include::modules/oadp-using-enable-api-group-versions.adoc[leveloffset=+2] | ||
|
|
||
| [id="backing-up-data-one-cluster-restoring-another-cluster"] | ||
| == Backing up data from one cluster and restoring it to another cluster |
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.
Similar comment as above -- it would be good to add a short introduction here.
|
|
||
| [id="oadp-storage-class-mapping"] | ||
| == OADP storage class mapping | ||
|
|
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.
Similar comment as above -- it would be good to add a short introduction here.
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
|
The This is because your PR targets the If the update in your PR does NOT apply to version 4.21 onward, please re-target this PR to go directly into the appropriate version branch or branches (enterprise-4.x) instead of main. |
Thank you @anahas-redhat ! |
|
/remove-lifecycle stale |
|
/label merge-review-needed |
| [id="openshift-adp-controller-manager-seg-fault_{context}"] | ||
| = OpenShift ADP Controller segmentation fault | ||
|
|
||
| If you configure a DPA with both `cloudstorage` and `restic` enabled, the `openshift-adp-controller-manager` pod crashes and restarts indefinitely until the pod fails with a crash loop segmentation fault. |
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.
*ADP
You need nouns after the literals cloudstorage and restic unless they are values.
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.
One comment to address.
Hi @JoeAldinger , Thank you for your comment. I'll inform the OADP team but I'm not allowed to make any content change in OADP documentation. It's beyond the scope of this PR. Could you merge this PR please? |
| // | ||
| // applications/projects/working-with-projects.adoc | ||
|
|
||
| :_mod-docs-content-type: PROCEDURE |
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.
This doesn't look like a procedure module to me. Is it a concept?
Hi this actually belongs to @anarnold97 and the aligned team. The PR has a few deal breakers from OCP's perspective including 3 commits that need squashed, a QE ack for the procedure and concept module, and I don't think either new module follows our guidelines for a procedure (there aren't any steps) and concept module (i.e. a NOTE callout isn't a concept module). Get with Andy and he can help you. |
|
@shdeshpa07 - please can you have a look at this PR and give @Pkylas007 some guidance Thanks |
Hi @anarnold97 and @Pkylas007 - Would it be better if I just take over the PR, work through the review comments and merge it after? Also, does this PR cover all the work for the DITA work for OADP doc set 1? If so, I will also back port it to |
Hi @shdeshpa07 This PR should not contain changes to content because it's a part of a specific modularization effort. However, if you think that's what OCP needs, then please go ahead: make content changes. Thanks! DITA work for set 1 has only this PR. |
@Pkylas007 - Thanks. You are correct. THere should be no content changes. I am yet to look into detail what the specific comments are. But, apart from the content changes, is there is any specific guidance you need from me? If not, could you squash the commits please, so I can do a quick review and merge it :). |
Fixed validation errors Update modules/oadp-adp-controller-segmentation.adoc Co-authored-by: Andy Arnold <[email protected]> Removed asciidoctor files
7cc5cf7 to
f25dce0
Compare
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
@shdeshpa07 Thank you :) I've squashed the commits. |
|
@Pkylas007: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@shdeshpa07 The include statements in the modules added in this PR have the necessary 1-line spacing in the main branch. Hence, I'm closing this PR as it is redundant. Thanks for looking into this! |
Version(s):
Issue:
Link to docs preview:
QE review:
Note -
As discussed with Kathryn, skipping the QE review because this PR focuses on adding a space after the include statements in modules and there are no technical changes.
Additional information: