-
Notifications
You must be signed in to change notification settings - Fork 1.8k
CNV-66694: Split for DITA compatability #102423
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
base: main
Are you sure you want to change the base?
Conversation
|
@MirzWeiss: This pull request references CNV-66694 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@MirzWeiss: This pull request references CNV-66694 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@MirzWeiss: all tests passed! 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. |
nunnatsa
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.
thanks @MirzWeiss .
I think it looks very good and clear now.
|
/lgtm |
|
@MirzWeiss: This pull request references CNV-66694 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
abrennan89
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.
First module looks good 🙂
For the second procedure module, perhaps consider making this a reference module instead. It seems like more of a list of possible commands that you can run to achieve different outcomes than a step by step procedure?
| ==== | ||
| You can also collect `must-gather` logs for all Operators and products on your cluster by running following command: | ||
| [source,terminal,subs="attributes+"] | ||
| ---- | ||
| $ oc adm must-gather --all-images | ||
| ---- | ||
| ==== |
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 would honestly leave this note out. I'm not sure what the use case would be for getting this information, and since procedures are supposed to accomplish a single task, I think this might be superfluous.
| .. Run the following command to modify the number of processes running in parallel when collecting `must-gather` data: | ||
| + | ||
| [source,terminal,subs="attributes+"] | ||
| ---- | ||
| $ oc adm must-gather \ | ||
| --image=registry.redhat.io/container-native-virtualization/cnv-must-gather-rhel9:v{HCOVersion} \ | ||
| -- PROS=<number> /usr/bin/gather | ||
| ---- | ||
| + | ||
| `PROS` defines the number of parallel processes running to collect data. The default number of processes is 5. Increasing the number of processes may result in faster data collection, but uses more resources. Increasing the maximum number of parallel processes is not recommended. | ||
| .. Run the following command to collect detailed information for a specific VM in a specific namespace: | ||
| + | ||
| [source,terminal,subs="attributes+"] | ||
| ---- | ||
| $ oc adm must-gather \ | ||
| --image=registry.redhat.io/container-native-virtualization/cnv-must-gather-rhel9:v{HCOVersion} \ | ||
| -- NS=<namespace name> VM=<VM name> /usr/bin/gather --vms_details | ||
| ---- | ||
| + | ||
| `NS` is the environment variable for `namespace`. It is mandatory when using the `VM` environment variable. | ||
| .. Run the following command to collect image, image-stream, and image-stream-tags information from the cluster: | ||
| + | ||
| [source,terminal,subs="attributes+"] | ||
| ---- | ||
| $ oc adm must-gather \ | ||
| --image=registry.redhat.io/container-native-virtualization/cnv-must-gather-rhel9:v{HCOVersion} \ | ||
| /usr/bin/gather --images | ||
| ---- | ||
| .. Run the following command to collect information about instance types from the cluster: | ||
| + | ||
| [source,terminal,subs="attributes+"] | ||
| ---- | ||
| $ oc adm must-gather \ | ||
| --image=registry.redhat.io/container-native-virtualization/cnv-must-gather-rhel9:v{HCOVersion} \ | ||
| /usr/bin/gather --instancetypes | ||
| ---- |
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 to me reads more like a reference document than a procedure. I would maybe update the type and structure to that instead, since it looks like these are all different optional commands that achieve specific use cases?
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.
SOB
That's how it was to begin with! But it was added to the contentX stuff as needing to be split up into separate modules because this was considered 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.
😂 😭
I think it's the steps / bullet formatting that make it procedure.
IMHO if we had just the commands and laid them out as example commands in a paragraph explaining usage it would be better as reference.
I think to truly fit the procedure structure, they would each need to be individual procedures, since each one of these steps is accomplishing a different, unrelated task, with a different user story. E.g. "Collecting information about instance types from the cluster", "Collecting information for a specific VM in a namespace"
Version(s):
Main, 4.20, 4.21
Issue:
CNV-66694
Link to docs preview
QE review: