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

RN-CNV-5472 Supported Guest OS Templates #23741

Merged
merged 1 commit into from Jul 15, 2020

Conversation

lmandavi
Copy link
Contributor

This PR is associated with https://issues.redhat.com/browse/CNV-5472.

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 13, 2020
@lmandavi
Copy link
Contributor Author

@fabiand @oyahud - Please review and add LGTM so that this PR can move to peer review stage.

@omeryahud
Copy link

/lgtm

@openshift-ci-robot
Copy link

@omeryahud: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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/test-infra repository.

@lmandavi
Copy link
Contributor Author

Feedback received from Fabian via Google Docs was incorporated. QE review by @omeryahud is complete. Requesting peer review.

@lmandavi
Copy link
Contributor Author

Requesting peer review.

Copy link
Member

@ousleyp ousleyp left a comment

Choose a reason for hiding this comment

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

I have a few suggestions for this one; let me know if you have questions. Thanks, @lmandavi!

virt/virt-2-4-release-notes.adoc Outdated Show resolved Hide resolved
virt/virt-2-4-release-notes.adoc Outdated Show resolved Hide resolved
@ousleyp ousleyp added the peer-review-done Signifies that the peer review team has reviewed this PR label Jul 14, 2020
Copy link
Contributor

@fabiand fabiand left a comment

Choose a reason for hiding this comment

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

Fine once Pan's comment is getting addressed.

virt/virt-2-4-release-notes.adoc Outdated Show resolved Hide resolved
@lmandavi lmandavi force-pushed the RN-CNV-5472-Guest-OS branch 2 times, most recently from 8ea4f22 to 7dccb5b Compare July 14, 2020 20:17
@lmandavi
Copy link
Contributor Author

@ousleyp Please review. @fabiand Made the changes.

Copy link
Member

@ousleyp ousleyp left a comment

Choose a reason for hiding this comment

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

A couple nits, but this is looking better.

Comment on lines 30 to 34
{VirtProductName} guests can use the following operating systems:

** Red Hat Enterprise Linux 6, 7, 8.
** Microsoft Windows Server 2012 R2, 2016, and 2019.
** Microsoft Windows 10.
Copy link
Member

Choose a reason for hiding this comment

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

  • Please change the **s to * now that you've removed the initial bullet point.
  • Please add and: * Red Hat Enterprise Linux 6, 7, and 8. (parallel structure with the next line)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ousleyp Requesting clarification, I didn't understand the comments. Can you show how you want the bullets and the text to be displayed (the complete structure). Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Content was updated.

** Red Hat Enterprise Linux 6, 7, 8.
** Microsoft Windows Server 2012 R2, 2016, and 2019.
** Microsoft Windows 10.
+
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the + so that this statement aligns with the intro sentence (now that the initial bullet point has been removed).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ousleyp Request clarification. I didn't understand this comment. Please show the actual structure of you want it. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

For both of these comments, please see if the asciidoc reference clarifies things: https://asciidoctor.org/docs/asciidoc-syntax-quick-reference/#lists

Regarding the +, see the "list continuation" example in the section titled "Complex content in outline lists" and let me know if you still have questions. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Content was updated.

This is a combination of 2 commits July 14 afternoon.

This is a combination of 2 commits on July 14.

This is a combination of 3 commits on July 14.

initial draft

remove fedora

bullet order

content reorder

remove redundant quote mark

peer review input
@lmandavi
Copy link
Contributor Author

PR is ready to be merged. Thank you!

@ousleyp ousleyp merged commit f3d0539 into openshift:enterprise-4.5 Jul 15, 2020
@lmandavi lmandavi deleted the RN-CNV-5472-Guest-OS branch July 15, 2020 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
peer-review-done Signifies that the peer review team has reviewed this PR size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants