Skip to content

Comments

Fix existing PR check failures#11

Merged
sutaakar merged 2 commits intoopendatahub-io:mainfrom
sutaakar:pr-check
Sep 24, 2025
Merged

Fix existing PR check failures#11
sutaakar merged 2 commits intoopendatahub-io:mainfrom
sutaakar:pr-check

Conversation

@sutaakar
Copy link
Collaborator

@sutaakar sutaakar commented Sep 24, 2025

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):
Fixes #

Checklist:

  • Docs included if any changes are user facing

Summary by CodeRabbit

  • Chores

    • Simplified Go CI workflow by removing a redundant finalization step, streamlining test coverage handling. No impact on product behavior.
  • Style

    • Standardized end-of-file newlines and minor whitespace across Dockerfile and multiple YAML manifests (runtime and metadata files), improving consistency and maintainability without changing functionality.

No user-facing features, behavior, or configurations were altered.

@coderabbitai
Copy link

coderabbitai bot commented Sep 24, 2025

Walkthrough

Removed the "finish" job from the Go CI workflow; other changes are whitespace/newline formatting updates across Dockerfile and YAML manifests without semantic impact.

Changes

Cohort / File(s) Summary of Changes
Go CI workflow
.github/workflows/test-go.yaml
Removed the separate finish job that ran goveralls with parallel-finished; no other job/step changes.
Trainer controller Dockerfile
cmd/trainer-controller-manager/Dockerfile.multiarch
Added trailing newline at EOF; no functional changes.
Component metadata
manifests/component_metadata.yaml
Added trailing newline; no content changes.
RHOAI runtime manifests formatting
manifests/rhoai/runtimes/kustomization.yaml, manifests/rhoai/runtimes/torch_cuda_241.yaml, manifests/rhoai/runtimes/torch_cuda_251.yaml, manifests/rhoai/runtimes/torch_rocm_241.yaml
Whitespace/EOF newline adjustments and blank-line removal; kustomization.yaml net effect unchanged; no semantic changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant GH as GitHub Actions
  participant Test as test-go job
  %% Removed participant Finish as finish job (goveralls parallel-finished)

  Dev->>GH: Push / PR
  GH->>Test: Trigger tests and coverage (parallel)
  Note right of Test: Runs Go tests and coverage steps

  %% Previously:
  rect rgba(200,200,255,0.2)
  Note over GH,Test: Previously, a dependent "finish" job ran goveralls parallel-finished
  end

  GH-->>Dev: Report test status
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paw: one job hops away,
The finish line moved—still green today.
Newlines tucked, YAMLs smoothed just so,
Docker hums softly, ready to go.
In CI fields, I nibble bugs for fun—
Fewer steps, same run. ʕ•ᴥ•ʔ🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Fix existing PR check failures" concisely conveys the intent to address CI/PR check problems and is directly related to the primary change in the diff (removing the separate Go workflow "finish" job), while other edits are formatting-only; it is relevant and clear enough for reviewers to understand the PR purpose. Although somewhat generic, it accurately reflects the main objective.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sutaakar sutaakar marked this pull request as ready for review September 24, 2025 12:26
Copy link
Member

@abhijeet-dhumal abhijeet-dhumal left a comment

Choose a reason for hiding this comment

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

/lgtm
Thanks Karel ! 🙌

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
cmd/trainer-controller-manager/Dockerfile.multiarch (1)

7-7: Pin the base image instead of using latest for reproducibility.

Avoid mutable tags to ensure repeatable builds and supply‑chain hygiene.

Apply:

-FROM registry.access.redhat.com/ubi9/ubi-minimal:latest
+FROM registry.access.redhat.com/ubi9/ubi-minimal:9.4

If possible, pin to a digest (preferred) for maximum immutability.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ffb67b and 9ce1707.

📒 Files selected for processing (7)
  • .github/workflows/test-go.yaml (0 hunks)
  • cmd/trainer-controller-manager/Dockerfile.multiarch (1 hunks)
  • manifests/component_metadata.yaml (1 hunks)
  • manifests/rhoai/runtimes/kustomization.yaml (1 hunks)
  • manifests/rhoai/runtimes/torch_cuda_241.yaml (0 hunks)
  • manifests/rhoai/runtimes/torch_cuda_251.yaml (0 hunks)
  • manifests/rhoai/runtimes/torch_rocm_241.yaml (0 hunks)
💤 Files with no reviewable changes (4)
  • manifests/rhoai/runtimes/torch_rocm_241.yaml
  • .github/workflows/test-go.yaml
  • manifests/rhoai/runtimes/torch_cuda_241.yaml
  • manifests/rhoai/runtimes/torch_cuda_251.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T10:34:26.025Z
Learnt from: sutaakar
PR: opendatahub-io/trainer#5
File: manifests/component_metadata.yaml:1-4
Timestamp: 2025-09-02T10:34:26.025Z
Learning: In component_metadata.yaml files for OpenDataHub projects, the repoUrl should point to the upstream Kubeflow repository (e.g., https://github.com/kubeflow/trainer) rather than the opendatahub-io fork, as this represents the upstream source of the component.

Applied to files:

  • manifests/component_metadata.yaml
🔇 Additional comments (2)
manifests/rhoai/runtimes/kustomization.yaml (1)

7-7: Resource entry verified — file exists and is referenced
manifests/rhoai/runtimes/torch_rocm_251.yaml is present and referenced in manifests/rhoai/runtimes/kustomization.yaml:7.

manifests/component_metadata.yaml (1)

4-4: Repo URL points to upstream Kubeflow — LGTM (v2.0.0 tag verified).
Upstream repo https://github.com/kubeflow/trainer contains tag v2.0.0.

Copy link
Collaborator

@ChughShilpa ChughShilpa left a comment

Choose a reason for hiding this comment

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

/lgtm

@sutaakar sutaakar merged commit d0c78f5 into opendatahub-io:main Sep 24, 2025
5 checks passed
@sutaakar sutaakar deleted the pr-check branch September 24, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants