-
Notifications
You must be signed in to change notification settings - Fork 429
OTA-1856: pkg/cli/admin/release/mirror: Deprecate in favor of oc-mirror #2195
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
|
@wking: This pull request references OTA-1856 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 story to target the "4.22.0" version, but no target version was set. DetailsIn 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. |
WalkthroughAdded a Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
b9ef5c0 to
92e6968
Compare
|
Testing with 92e6968: $ ./oc adm release mirror --help >/dev/null
Command "mirror" is deprecated, with deprecation (but continued support) in 4.22 with removal planned for 5.0. Please use oc-mirror instead. |
92e6968 to
a577c3a
Compare
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pkg/cli/admin/release/mirror.go`:
- Around line 96-99: Update the Deprecated message on the mirror command to
match the intended removal version: locate the cobra command definition that has
the Deprecated field (the command with Use: "mirror", Short: "Mirror a release
to a different image registry location") and change the text from "with
deprecation (but continued support) in 4.22 with removal planned for 5.1.
Please use oc-mirror instead." to use "removal planned for 5.0" so the
deprecation timeline is consistent with the PR and CLI output.
pkg/cli/admin/release/mirror.go
Outdated
| Use: "mirror", | ||
| Short: "Mirror a release to a different image registry location", | ||
| Deprecated: "with deprecation (but continued support) in 4.22 with removal planned for 5.1. Please use oc-mirror instead.", | ||
| Long: templates.LongDesc(` |
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.
Align deprecation timeline with stated intent (Line 98).
PR objectives and reported CLI output indicate removal planned for 5.0, but the message says 5.1. Please update the version to avoid user-facing inconsistencies.
Proposed fix
- Deprecated: "with deprecation (but continued support) in 4.22 with removal planned for 5.1. Please use oc-mirror instead.",
+ Deprecated: "with deprecation (but continued support) in 4.22 with removal planned for 5.0. Please use oc-mirror instead.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Use: "mirror", | |
| Short: "Mirror a release to a different image registry location", | |
| Deprecated: "with deprecation (but continued support) in 4.22 with removal planned for 5.1. Please use oc-mirror instead.", | |
| Long: templates.LongDesc(` | |
| Use: "mirror", | |
| Short: "Mirror a release to a different image registry location", | |
| Deprecated: "with deprecation (but continued support) in 4.22 with removal planned for 5.0. Please use oc-mirror instead.", | |
| Long: templates.LongDesc(` |
🤖 Prompt for AI Agents
In `@pkg/cli/admin/release/mirror.go` around lines 96 - 99, Update the Deprecated
message on the mirror command to match the intended removal version: locate the
cobra command definition that has the Deprecated field (the command with Use:
"mirror", Short: "Mirror a release to a different image registry location") and
change the text from "with deprecation (but continued support) in 4.22 with
removal planned for 5.1. Please use oc-mirror instead." to use "removal planned
for 5.0" so the deprecation timeline is consistent with the PR and CLI output.
Reduce the number of use-facing commands that need to be documented and maintained. oc-mirror provides a more comprehensive experience, including mirroring OLM catalogs and Sigstore signatures and other functionality that is necessary in most OpenShift deployments.
a577c3a to
84e5849
Compare
|
@wking: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
ardaguclu
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.
Considering that oc-mirror covers feature parity of this, I think, this is the right move.
| Short: "Mirror a release to a different image registry location", | ||
| Use: "mirror", | ||
| Short: "Mirror a release to a different image registry location", | ||
| Deprecated: "with deprecation (but continued support) in 4.22 with removal planned for 5.1. Please use oc-mirror instead.", |
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.
In order to align with other deprecation messages, what about this;
| Deprecated: "with deprecation (but continued support) in 4.22 with removal planned for 5.1. Please use oc-mirror instead.", | |
| Deprecated: "and will be removed in the future (5.1). Use oc-mirror instead.", |
|
Test Scenario: verify warning is present in Step1: Launch cluster with changes. Step2: verify warning is present with |
|
/verified by @dis016 |
Reduce the number of use-facing commands that need to be documented and maintained. oc-mirror provides a more comprehensive experience, including mirroring OLM catalogs and Sigstore signatures and other functionality that is necessary in most OpenShift deployments.