Skip to content

Conversation

@rolfedh
Copy link
Contributor

@rolfedh rolfedh commented Sep 27, 2020

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 27, 2020
@rolfedh rolfedh changed the title [WIP] OCPRHV-391 Add not tested/not supported notice to OCP 4.6 and RHV 4.3 docs [enterprise-4.6] [WIP] OCPRHV-391 Add not tested/not supported notice to OCP 4.6 and RHV 4.3 docs [enterprise-4.6][enterprise-4.5] Sep 28, 2020
@rolfedh rolfedh changed the title [WIP] OCPRHV-391 Add not tested/not supported notice to OCP 4.6 and RHV 4.3 docs [enterprise-4.6][enterprise-4.5] [WIP] OCPRHV-391 Add not tested/not supported notice to OCP 4.6 and RHV 4.3 docs [enterprise-4.6][enterprise-4.5][enterprise-4.4] Sep 30, 2020
Copy link
Member

@sandrobonazzola sandrobonazzola left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 30, 2020
@sandrobonazzola sandrobonazzola removed their assignment Sep 30, 2020
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 4, 2020
@pneedle-rh pneedle-rh self-requested a review October 5, 2020 07:35
@pneedle-rh pneedle-rh added the peer-review-needed Signifies that the peer review team needs to review this PR label Oct 5, 2020
Copy link
Contributor

@pneedle-rh pneedle-rh left a comment

Choose a reason for hiding this comment

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

@rolfedh I have now completed my peer review for this PR. Please let me know if you have any questions about my suggestions.

Can you please also confirm which OCP releases this PR relates to? Is this just for master and branch/enterprise-4.6? Thanks!

@pneedle-rh pneedle-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Oct 5, 2020
@rolfedh rolfedh changed the title [WIP] OCPRHV-391 Add not tested/not supported notice to OCP 4.6 and RHV 4.3 docs [enterprise-4.6][enterprise-4.5][enterprise-4.4] OCPRHV-391 Add not tested/not supported notice to OCP 4.6 and RHV 4.3 docs [enterprise-4.6][enterprise-4.5][enterprise-4.4] Oct 5, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 5, 2020
@pneedle-rh pneedle-rh merged commit cc95b30 into openshift:master Oct 5, 2020
@pneedle-rh
Copy link
Contributor

/cherrypick enterprise-4.4

@openshift-cherrypick-robot

@pneedle-rh: #25821 failed to apply on top of branch "enterprise-4.4":

Applying: Update
Using index info to reconstruct a base tree...
M	installing/installing_rhv/installing-rhv-customizations.adoc
M	installing/installing_rhv/installing-rhv-default.adoc
M	modules/installation-initializing.adoc
A	virt/virtual_machines/importing_vms/virt-importing-rhv-vm.adoc
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): virt/virtual_machines/importing_vms/virt-importing-rhv-vm.adoc deleted in HEAD and modified in Update. Version Update of virt/virtual_machines/importing_vms/virt-importing-rhv-vm.adoc left in tree.
Auto-merging modules/installation-initializing.adoc
CONFLICT (content): Merge conflict in modules/installation-initializing.adoc
Auto-merging installing/installing_rhv/installing-rhv-default.adoc
CONFLICT (content): Merge conflict in installing/installing_rhv/installing-rhv-default.adoc
Auto-merging installing/installing_rhv/installing-rhv-customizations.adoc
CONFLICT (content): Merge conflict in installing/installing_rhv/installing-rhv-customizations.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Update
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.4

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.

@pneedle-rh
Copy link
Contributor

/cherrypick enterprise-4.5

@openshift-cherrypick-robot

@pneedle-rh: new pull request created: #26109

Details

In response to this:

/cherrypick enterprise-4.5

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.

@pneedle-rh
Copy link
Contributor

/cherrypick enterprise-4.6

@openshift-cherrypick-robot

@pneedle-rh: #25821 failed to apply on top of branch "enterprise-4.6":

Applying: Update
Using index info to reconstruct a base tree...
M	installing/installing_rhv/installing-rhv-customizations.adoc
M	installing/installing_rhv/installing-rhv-default.adoc
M	modules/installation-initializing.adoc
M	virt/virtual_machines/importing_vms/virt-importing-rhv-vm.adoc
Falling back to patching base and 3-way merge...
Auto-merging virt/virtual_machines/importing_vms/virt-importing-rhv-vm.adoc
Auto-merging modules/installation-initializing.adoc
CONFLICT (content): Merge conflict in modules/installation-initializing.adoc
Auto-merging installing/installing_rhv/installing-rhv-default.adoc
CONFLICT (content): Merge conflict in installing/installing_rhv/installing-rhv-default.adoc
Auto-merging installing/installing_rhv/installing-rhv-customizations.adoc
CONFLICT (content): Merge conflict in installing/installing_rhv/installing-rhv-customizations.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Update
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.6

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.

@pneedle-rh
Copy link
Contributor

@rolfedh I have merged this PR to master and the openshift-cherrypick-robot successfully created a CP PR for 4.5, which I have just merged. However, there were several merge conflicts when trying to cherry pick to 4.4 and 4.6.

  • For 4.4, one of the conflicts relates to virt/virtual_machines/importing_vms/virt-importing-rhv-vm.adoc not existing in 4.4 prior to this CP. Does this mean that you need to add the warning to the CNV named equivalent file in 4.4 instead? There were three other conflicts also. I think the manual cherry picks will benefit from your experience, since they are a little involved.

  • For 4.6, the merge conflicts look like they relate to this earlier PR. Going by this comment, it seems that the warning introduced by that earlier PR is not intended for 4.6. It was introduced into 4.4 and 4.5, but 4.6. The current PR 4.6 CP is trying to edit that warning although it does not exist in the 4.6 release branch.

Please can you do the following?:

  1. Manually cherry pick this current PR for 4.4 and 4.6. For 4.6 I think that requires manually removing that warning from the conflicted files.

  2. Create an additional PR that removes the warning from master, to prevent future merge conflicts.

  3. @bergerhoffer noticed that this instance of the warning also needs attributes instead of 'OpenShift Container Platform', 'OCP' and 'Red Hat Virtualization (RHV)'. My apologies, I missed this in my review. Can you please apply attributes to that comment in another PR?

Thank you!

@rolfedh
Copy link
Contributor Author

rolfedh commented Oct 8, 2020

I'm stuck. @pneedle-rh I don't have privileges to push (cherry picked) changes to enterprise-4.x branches. (Or I just don't know how to do this.) Can you give me some advice or direct me to someone for help?

@pneedle-rh
Copy link
Contributor

I'm stuck. @pneedle-rh I don't have privileges to push (cherry picked) changes to enterprise-4.x branches. (Or I just don't know how to do this.) Can you give me some advice or direct me to someone for help?

@rolfedh thank you for setting up a call. I look forward to discussing this with you later today.

@mburke5678
Copy link
Contributor

mburke5678 commented Feb 4, 2021

@pneedle-rh @rolfedh It appears that Paul's requests in #25821 (comment) never got implemented, is that true? Do you still plan to CP to 4.4 and 4.6?
If so, can you CP to 4.7 also?

I have a PR to remove the second note from the OKD docs (which isn't really needed as the entire topic isn't in OKD).
#25821 (comment)

@rolfedh
Copy link
Contributor Author

rolfedh commented Feb 4, 2021 via email

@pneedle-rh
Copy link
Contributor

pneedle-rh commented Feb 9, 2021

@rolfedh I should imagine that the same merge conflict will arise for 4.7 as was seen for 4.6. As per my earlier comment for 4.6, I think it will be necessary to manually cherry pick for 4.7 (as well as 4.4 and 4.6) and resolve the points that I outlined.

Please let me know if you would like to run through this on a screen share.

@rolfedh
Copy link
Contributor Author

rolfedh commented Feb 11, 2021

This customer story has been addressed in 4.7 by #29364

@rolfedh rolfedh deleted the OCPRHV-391-note-no-ocp46onrhv43 branch February 14, 2021 15:44
@rolfedh rolfedh restored the OCPRHV-391-note-no-ocp46onrhv43 branch February 14, 2021 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.4 branch/enterprise-4.5 branch/enterprise-4.6 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.

6 participants