ci: add CODEOWNERS and fix Unit Tests check for required status checks#104
Conversation
Assigns agentic-starter-kits-maintainers as code owners for all files, enabling the require_code_owner_review rule in the repo-level ruleset. Ref: RHAIENG-4065 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a repository-wide CODEOWNERS rule assigning ChangesRepository ownership
Agent tests workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The repo-level ruleset requires the "Unit Tests" check to pass before merge. With path filtering, PRs that don't touch agents/ never ran the workflow, leaving the check pending indefinitely. Replace the path filter with dorny/paths-filter and add a skip job that reports success when no agent files changed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/agent-tests.yml (1)
115-123: ⚡ Quick winConsider documenting the duplicate job name pattern.
Both the
testandskipjobs share the name "Unit Tests" to ensure that one always runs and reports to GitHub's required status check, preventing it from remaining pending. While this is a known workaround pattern and the mutually exclusive conditions ensure correctness, it can be confusing for maintainers unfamiliar with this approach.📝 Suggested improvement: Add a clarifying comment
+ # Note: Both 'test' and 'skip' jobs are named "Unit Tests" so that one always runs + # and satisfies the required status check, preventing it from staying pending. skip: name: Unit Tests needs: changes if: needs.changes.outputs.agents != 'true'🤖 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 @.github/workflows/agent-tests.yml around lines 115 - 123, Add a short clarifying comment to the GitHub Actions workflow explaining why the 'test' and 'skip' jobs both use the same name "Unit Tests" (the duplicate-name pattern ensures a required status check is always reported and prevents a pending check when one branch of the conditional is skipped); place this comment near the job definitions for the 'test' and 'skip' jobs in agent-tests.yml so future maintainers see the rationale next to the name "Unit Tests" and the conditional logic (needs.changes.outputs.agents and if conditions).
🤖 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.
Nitpick comments:
In @.github/workflows/agent-tests.yml:
- Around line 115-123: Add a short clarifying comment to the GitHub Actions
workflow explaining why the 'test' and 'skip' jobs both use the same name "Unit
Tests" (the duplicate-name pattern ensures a required status check is always
reported and prevents a pending check when one branch of the conditional is
skipped); place this comment near the job definitions for the 'test' and 'skip'
jobs in agent-tests.yml so future maintainers see the rationale next to the name
"Unit Tests" and the conditional logic (needs.changes.outputs.agents and if
conditions).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: e0187807-9872-4d5a-a3fe-b86556799e5c
📒 Files selected for processing (1)
.github/workflows/agent-tests.yml
Two jobs with the same display name caused duplicate checks on PRs. Consolidate into a single "Unit Tests" job that always runs but skips the test steps when no agent files changed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Description
Two changes to support the new repo-level ruleset ("Require reviews"):
Adds
.github/CODEOWNERS— assigns@red-hat-data-services/agentic-starter-kits-maintainersas code owners for all files, activating therequire_code_owner_reviewrule so at least one team member must approve every PR.Updates
agent-tests.yml— the ruleset requires theUnit Testscheck to pass, but the workflow previously used path-based triggers (paths: ['agents/**']), so PRs not touching agent files never reported the check, leaving it pending indefinitely. The workflow now always runs but skips the expensive test steps (Python setup, uv, test execution) when noagents/files changed.Jira Ticket
RHAIENG-4065
Testing
After merge:
Unit Testscheck passes (skipped) and review is requested fromagentic-starter-kits-maintainersagents/and confirm tests actually runChecklist
.envor secret files are included in this PRReview Guidance
.github/CODEOWNERS— single line, confirm the team name matches the GitHub team.github/workflows/agent-tests.yml— removedpathstrigger, added achangesstep that setsagents=trueonly whenagents/files are modified; all subsequent steps are gated on that outputRelated PRs
agent-tests.ymlworkflow (theUnit Testscheck now required by the ruleset)