OCPCLOUD-3513: feat: add gh-summary target to download manifests from GitHub#617
OCPCLOUD-3513: feat: add gh-summary target to download manifests from GitHub#617hongkailiu wants to merge 1 commit into
Conversation
|
@hongkailiu: This pull request references OCPCLOUD-3513 which is a valid 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe ChangesGitHub manifest download in verify
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@openshift/Makefile`:
- Around line 9-20: The gh-summary target uses REPO_OWNER, REPO_NAME, and
PULL_BASE_SHA environment variables in the curl command without validating they
are set first, causing unclear error messages when running locally where these
variables are empty. Add conditional checks at the beginning of the gh-summary
target to verify that all three variables (REPO_OWNER, REPO_NAME, and
PULL_BASE_SHA) are defined and non-empty before executing the curl command, and
print a clear error message indicating which variable is missing if the check
fails.
- Line 16: The MAKEFILE_DIR assignment that uses $(dir $(abspath $(lastword
$(MAKEFILE_LIST)))) is currently positioned after the include
provider-version.mk statement at line 4. This causes the assignment to capture
provider-version.mk as the last file in MAKEFILE_LIST instead of
openshift/Makefile. Move the MAKEFILE_DIR assignment to occur before the include
provider-version.mk statement so that when the variable is evaluated, the
lastword in MAKEFILE_LIST will correctly reference the main Makefile rather than
the included file.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f3e01209-81d7-42e5-8f37-2dc02217d17f
📒 Files selected for processing (1)
openshift/Makefile
Add a new Makefile target that downloads the manifests-summary.yaml file from a specific commit in the GitHub repository using REPO_OWNER, REPO_NAME, and PULL_BASE_SHA environment variables. Those environment variables will be expose by [Prow](https://docs.prow.k8s.io/docs/jobs/#job-environment-variables). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Hongkai Liu <hongkailiu@users.noreply.github.com>
|
From this job: The env. variables have the expected values. |
|
@hongkailiu: The following tests failed, say
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. |
Add a new Makefile target that downloads the manifests-summary.yaml file from a specific commit in the GitHub repository using REPO_OWNER, REPO_NAME, and PULL_BASE_SHA environment variables.
Those environment variables will be expose by Prow.
/hold
Require #616 to go first.
Summary by CodeRabbit