Skip to content

Conversation

@sbeskin-redhat
Copy link
Contributor

@sbeskin-redhat sbeskin-redhat commented Jun 17, 2024

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 17, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jun 17, 2024

@sbeskin-redhat: This pull request references CNV-33890 which is a valid jira issue.

In response to this:

Resolves: https://issues.redhat.com/browse/CNV-33888

OCP 4.16
CNV 4.16

Preview:

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.

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

ocpdocs-previewbot commented Jun 17, 2024

🤖 Fri Jul 05 14:27:54 - Prow CI generated the docs preview:

https://77535--ocpdocs-pr.netlify.app/openshift-enterprise/latest/virt/release_notes/virt-4-16-release-notes.html

@sbeskin-redhat
Copy link
Contributor Author

/retest

@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 17, 2024
@sbeskin-redhat sbeskin-redhat changed the title CNV-33890: Documentation regarding swap CNV-33890: Swap feature in CNV RN 4.16 Jun 17, 2024
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jun 25, 2024

@sbeskin-redhat: This pull request references CNV-33890 which is a valid jira issue.

In response to this:

Resolves: https://issues.redhat.com/browse/CNV-33888

OCP 4.16
CNV 4.16

Preview: https://77535--ocpdocs-pr.netlify.app/openshift-enterprise/latest/virt/release_notes/virt-4-16-release-notes.html#virt-4-16-technology-preview

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.

@sbeskin-redhat sbeskin-redhat force-pushed the 4_16_CNV_33890_swap_RN branch 2 times, most recently from ff4f06c to 1d3580f Compare June 26, 2024 14:04
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2024
@sbeskin-redhat
Copy link
Contributor Author

@fabiand
Sorry, I forgot to mention that this PR is only about the release notes: virt/release_notes/virt-4-16-release-notes.adoc
The other files relating to the swap feature (virt-configuring-higher-vm-workload-density.adoc and virt-using-wasp-agent-to-configure-higher-vm-workload-density.adoc) are updated in another PR: #76344

@sbeskin-redhat
Copy link
Contributor Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Jul 3, 2024
@max-cx
Copy link
Contributor

max-cx commented Jul 4, 2024

/label peer-review-in-progress

@openshift-ci openshift-ci bot added the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Jul 4, 2024
@max-cx
Copy link
Contributor

max-cx commented Jul 4, 2024

/remove-label peer-review-needed

@openshift-ci openshift-ci bot removed the peer-review-needed Signifies that the peer review team needs to review this PR label Jul 4, 2024
@max-cx
Copy link
Contributor

max-cx commented Jul 5, 2024

@sbeskin-redhat, 🙂 I'm looking at the preview and it seems that you already added the prerequisites but still need to add a procedure. Also, I see the previous reviewer's comments that have not been addressed or responded to. I also recommend consulting the repo rules files such as how to document TP features.

🙂 AFAIK, we normally get peer reviews when we feel our authoring work on the PR is completed, so I suggest that you add the missing procedure, get this PR to a point where you believe that it's ready to be merged, and ask the QE to review and approve it. Also, please remember to squash the commits. And then add the peer-review-needed label.

Since I'm only onboarding for the peer review squad, let's ask for a second opinion or additional feedback from @dfitzmau.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 5, 2024
@max-cx
Copy link
Contributor

max-cx commented Jul 5, 2024

/remove-label peer-review-in-progress

@openshift-ci openshift-ci bot removed the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Jul 5, 2024
@max-cx
Copy link
Contributor

max-cx commented Jul 5, 2024

Sorry, I forgot to mention that this PR is only about the release notes: virt/release_notes/virt-4-16-release-notes.adoc
The other files relating to the swap feature (virt-configuring-higher-vm-workload-density.adoc and virt-using-wasp-agent-to-configure-higher-vm-workload-density.adoc) are updated in another PR: #76344

@sbeskin-redhat, 🙂 in such a case, please remove the changes that should not be merged with this PR.

@dfitzmau
Copy link
Contributor

dfitzmau commented Jul 5, 2024

/label peer-review-in-progress

@openshift-ci openshift-ci bot added the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Jul 5, 2024
@dfitzmau
Copy link
Contributor

dfitzmau commented Jul 5, 2024

Hi @sbeskin-redhat . I agree with Max. The commits need to be squashed, non-intended files should be reverted, and the merge conflict(s) need to be addressed.

@dfitzmau
Copy link
Contributor

dfitzmau commented Jul 5, 2024

/remove-label peer-review-in-progress

@openshift-ci openshift-ci bot removed the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Jul 5, 2024
@sbeskin-redhat sbeskin-redhat force-pushed the 4_16_CNV_33890_swap_RN branch from 3f19039 to 8973756 Compare July 5, 2024 11:31
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 5, 2024
@openshift-ci openshift-ci bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 5, 2024
@sbeskin-redhat sbeskin-redhat force-pushed the 4_16_CNV_33890_swap_RN branch from 8973756 to bd1935e Compare July 5, 2024 11:32
@openshift-ci openshift-ci bot removed the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 5, 2024
@sbeskin-redhat sbeskin-redhat force-pushed the 4_16_CNV_33890_swap_RN branch from 42cb1f4 to f679d75 Compare July 5, 2024 11:55
@openshift-ci openshift-ci bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 5, 2024
@sbeskin-redhat sbeskin-redhat force-pushed the 4_16_CNV_33890_swap_RN branch 2 times, most recently from d79ca91 to f2c761c Compare July 5, 2024 12:00
@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 Jul 5, 2024
@sbeskin-redhat
Copy link
Contributor Author

@dfitzmau
Hi,
Squashed, rebased, resolved

Copy link
Contributor

@dfitzmau dfitzmau left a comment

Choose a reason for hiding this comment

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

/label peer-review-done

* Cluster admins can now enable CPU resource limits on a namespace in the {product-title} web console under *Overview* -> *Settings* -> *Preview features*.

//CNV-22314: Safe memory overcommitment using `wasp-agent`
* Cluster admins can now xref:../../virt/virtual_machines/virt-configuring-higher-vm-workload-density.adoc#virt-configuring-higher-vm-workload-density[use the `wasp-agent` tool to configure a higher VM workload density] in their clusters by overcommitting the amount of memory (RAM) and assigning swap resources to VM workloads.
Copy link
Contributor

Choose a reason for hiding this comment

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

For accessibility reasons I'm not sure inline linking is the way to go. How about step out the link on a new line:

For more information, see Configuring higher VM workload density.

This way when a customer clicks on the link it opens to a matching title.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dfitzmau I did it that way for uniformity - note that all other items have inline links. But if you strongly feel that a separate link is better, I will change it.

Copy link
Contributor

@dfitzmau dfitzmau Jul 5, 2024

Choose a reason for hiding this comment

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

I'll leave it to you, but for accessibility reasons, inline links can be tricky.

Screenshot from 2024-07-05 14-31-25

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dfitzmau I've left the link inline, but slightly modified the link text to better correspond to the heading of the section it leads to.

@openshift-ci openshift-ci bot added the peer-review-done Signifies that the peer review team has reviewed this PR label Jul 5, 2024
@sbeskin-redhat sbeskin-redhat force-pushed the 4_16_CNV_33890_swap_RN branch from e3981e2 to 8173fdc Compare July 5, 2024 14:18
@openshift-ci
Copy link

openshift-ci bot commented Jul 5, 2024

@sbeskin-redhat: 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.

@sbeskin-redhat
Copy link
Contributor Author

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Jul 5, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 7, 2024

@sbeskin-redhat: This pull request references CNV-33890 which is a valid jira issue.

In response to this:

Resolves: https://issues.redhat.com/browse/CNV-33890

OCP 4.16
CNV 4.16

Preview: https://77535--ocpdocs-pr.netlify.app/openshift-enterprise/latest/virt/release_notes/virt-4-16-release-notes.html#virt-4-16-technology-preview

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.

@sheriff-rh sheriff-rh added merge-review-in-progress Signifies that the merge review team is reviewing this PR and removed merge-review-needed Signifies that the merge review team needs to review this PR labels Jul 8, 2024
Copy link
Contributor

@sheriff-rh sheriff-rh left a comment

Choose a reason for hiding this comment

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

Changes LGTM, merging.

@sheriff-rh sheriff-rh added this to the Continuous Release milestone Jul 8, 2024
@sheriff-rh sheriff-rh merged commit 765d0bf into openshift:enterprise-4.16 Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.16 jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. merge-review-in-progress Signifies that the merge review team is reviewing this PR 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.

8 participants