NO-ISSUE: Fix latest RHOCP logic after 4.22 GA#6862
Conversation
WalkthroughTwo shell scripts update RHOCP repository detection and configuration: ChangesRHOCP Version Output and Repository Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/get-latest-rhocp-repo.sh (1)
26-41: ⚡ Quick winDuplicated logic with
configure-vm.sh.Both
LAST_MINOR_FOR_MAJORmap (Line 27 here, Line 197 in configure-vm.sh) andget_prev_version()function (Lines 31-41 here, Lines 201-211 in configure-vm.sh) are duplicated. When a new major version is released, both files need updating.Consider extracting this to a shared helper script sourced by both, or at minimum add a comment noting the duplication.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/get-latest-rhocp-repo.sh` around lines 26 - 41, The LAST_MINOR_FOR_MAJOR map and get_prev_version() function are duplicated across scripts; extract them into a single shared helper (e.g., a new script sourced by both) to avoid needing parallel updates, or at minimum add a clear TODO comment linking the duplicate locations; specifically move or centralize the declare -A LAST_MINOR_FOR_MAJOR and the get_prev_version() function (which sets prev_major and prev_minor) into the shared helper and update scripts to source that helper so only one source of truth must be changed when adding a new major version.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/devenv-builder/configure-vm.sh`:
- Line 219: The OCP_REPO_NAME assignment uses uname -i which can return
"unknown" on some VMs; change it to use uname -m for consistency with the
earlier usage and upstream script (update the
OCP_REPO_NAME="rhocp-${ver}-for-rhel-9-mirrorbeta-$(uname -i)-rpms" line to use
$(uname -m) so it matches the other occurrences and the get-latest-rhocp-repo.sh
behavior).
---
Nitpick comments:
In `@scripts/get-latest-rhocp-repo.sh`:
- Around line 26-41: The LAST_MINOR_FOR_MAJOR map and get_prev_version()
function are duplicated across scripts; extract them into a single shared helper
(e.g., a new script sourced by both) to avoid needing parallel updates, or at
minimum add a clear TODO comment linking the duplicate locations; specifically
move or centralize the declare -A LAST_MINOR_FOR_MAJOR and the
get_prev_version() function (which sets prev_major and prev_minor) into the
shared helper and update scripts to source that helper so only one source of
truth must be changed when adding a new major version.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b68f4e41-6a15-477e-b3ad-31c2bc9cc453
📒 Files selected for processing (2)
scripts/devenv-builder/configure-vm.shscripts/get-latest-rhocp-repo.sh
63ba33e to
c5a97cb
Compare
|
@pmtk: This pull request explicitly references no jira issue. 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. |
|
/lgtm |
|
/verified by CI |
|
@agullon: This PR has been marked as verified by 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: agullon, pmtk 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 |
|
@pmtk: 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. |
Summary by CodeRabbit