config/forge/resolve_job.yaml: sync with Forge Tasks - #104
Conversation
📝 WalkthroughWalkthroughThe resolve Job bootstrap script now configures workspace-based artifacts and logging, retrieves FournosJob settings, conditionally configures target-cluster access, resolves pull request or image commits, and receives ChangesResolve Job bootstrap
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: albertoperdomo2 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
config/forge/resolve_job.yaml (1)
46-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnquoted redirect target.
> $ARTIFACT_DIR/fournos_fjob.yamlshould be quoted for consistency with the quoting used elsewhere in this script (e.g. Lines 51, 67).🧹 Proposed fix
- oc get "fjob/$FJOB_NAME" -n "$FOURNOS_WORKLOAD_NAMESPACE" -oyaml > $ARTIFACT_DIR/fournos_fjob.yaml + oc get "fjob/$FJOB_NAME" -n "$FOURNOS_WORKLOAD_NAMESPACE" -oyaml > "$ARTIFACT_DIR/fournos_fjob.yaml"🤖 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 `@config/forge/resolve_job.yaml` at line 46, Quote the redirect target in the oc get command, using the existing ARTIFACT_DIR variable and fournos_fjob.yaml filename, to match the script’s other path handling.
🤖 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 `@config/forge/resolve_job.yaml`:
- Around line 38-39: Update the ARTIFACT_DIR assignment to use the declared
FOURNOS_STEP environment variable instead of the undefined FOURNOS_STEP_NAME,
preserving the existing artifact base directory and downstream artifact path
behavior under set -o nounset.
- Around line 89-97: Update the PULL_NUMBER branch’s PULL_PULL_SHA assignment to
run git rev-parse HEAD against "$FORGE_HOME", matching the existing fetch and
reset commands. Preserve exporting and logging the SHA from the updated PR
checkout.
- Around line 64-75: In export_env_var_from_fjob, separate the local declaration
of value from its command-substitution assignment so the cat/yq pipeline exit
status is preserved under errexit and pipefail. Keep the existing FournosJob
environment lookup and null-handling behavior unchanged.
---
Nitpick comments:
In `@config/forge/resolve_job.yaml`:
- Line 46: Quote the redirect target in the oc get command, using the existing
ARTIFACT_DIR variable and fournos_fjob.yaml filename, to match the script’s
other path handling.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10ededd8-bb9e-48c2-bca2-aea5dfc33599
📒 Files selected for processing (1)
config/forge/resolve_job.yaml
sync with https://github.com/openshift-psap/forge/blob/main/fournos/gitops/base/workflows/resolve-job.yaml
This should allow running jobs with PULL_NUMBER and no PULL_PULL_SHA, and use the HEAD commit of the PR
Summary by CodeRabbit
run.log.