ci: only run container job on distro, tests, or infra change#51
Conversation
|
Warning Rate limit exceeded@nathan-weinberg has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 58 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughUpdated Mergify configuration to switch PR merge method from squash to merge and removed two conflict-labeling actions. Adjusted a GitHub Actions workflow to add path filters to the pull_request trigger, aligning it with the push trigger. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub
participant WF as GH Actions: redhat-distro-container
participant MG as Mergify
Dev->>GH: Open/Update PR
GH->>WF: pull_request event (only if paths match)
Note over WF: Runs when changes in<br/>.github/actions/setup-vllm/action.yml,<br/>.github/workflows/redhat-distro-container.yml,<br/>distribution/**, tests/**
GH->>MG: PR status and rule evaluation
MG->>GH: Merge PR (method = merge)
Note over MG: Conflict label automation removed<br/>(no ping/no needs-rebase label)
Dev->>GH: Push to branches
GH->>WF: push event (same paths)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
39cf3a8 to
f3b0da4
Compare
cdoern
left a comment
There was a problem hiding this comment.
are single quotes necessary?
I must commend the thought and effort you've poured into this. Regrettably, I don't feel this is the correct direction for the project. |
f3b0da4 to
82cec90
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/redhat-distro-container.yml (1)
11-15: Pull request-only path filters; push still unfiltered — job will continue to run on every push.If the goal is to run the long job only on distro/tests/infra changes, mirror the same path filters under the push trigger.
Proposed change:
on: pull_request: branches: - main - rhoai-v* types: - opened - synchronize paths: - - '.github/actions/setup-vllm/action.yml' + - '.github/actions/setup-vllm/**' - '.github/workflows/redhat-distro-container.yml' - 'distribution/**' - 'tests/**' push: branches: - main - rhoai-v* + paths: + - '.github/actions/setup-vllm/**' + - '.github/workflows/redhat-distro-container.yml' + - 'distribution/**' + - 'tests/**'Notes:
- Expanded setup-vllm path to include the whole action directory to catch changes beyond action.yml.
- Confirm that publishing on pushes where only tests change is intended. The “Publish image” step runs on any push that meets the workflow trigger. If that’s not desired, we can gate publishing further (e.g., separate publish job with stricter paths, or a condition based on changed files).
- Single quotes around paths are optional; YAML and GitHub Actions accept these unquoted. If preferred, you can drop the quotes for consistency with common examples.
.github/mergify.yml (1)
17-17: Switch to merge commits — verify repo settings and desired history policy.Ensure “Allow merge commits” is enabled in repository settings and that this aligns with your history policy. Consider enabling strict/smart updates if you want Mergify to auto-update branches before merging to avoid outdated merges.
Example (if desired):
pull_request_rules: - name: auto-merge conditions: # ... actions: merge: method: merge strict: smart+fasttrack
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/mergify.yml(1 hunks).github/workflows/redhat-distro-container.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test-push (linux/amd64)
- GitHub Check: Summary
this is a long running job that doesn't need to run all the time Signed-off-by: Nathan Weinberg <nweinber@redhat.com>
Signed-off-by: Nathan Weinberg <nweinber@redhat.com>
82cec90 to
98d9e90
Compare
|
Merging manually so the Mergify change takes affect |
What does this PR do?
this is a long running job that doesn't need to run all the time
Summary by CodeRabbit
Chores
Tests