chore: improve AI bug automation readiness#53
chore: improve AI bug automation readiness#53davidebianchi merged 1 commit intoopendatahub-io:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces developer infrastructure and governance for the ODH CLI project. It adds a GitHub pull request template with structured sections for description, testing confirmation, and review checklist. A Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
OWNERS (1)
1-4: Single reviewer/approver creates review bottleneck.Same individual in both lists means no independent review enforcement. Consider adding at least one additional approver for bus-factor resilience when the team grows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@OWNERS` around lines 1 - 4, The OWNERS file currently lists the same person under both "reviewers" and "approvers" which creates a single-person bottleneck; update the YAML by adding at least one additional independent entry to "approvers" (and optionally "reviewers") so that "ugiordan" is not the sole approver—modify the "approvers:" list to include another team member's GitHub handle (or a team alias) alongside "ugiordan" to enforce independent approval.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.pre-commit-config.yaml:
- Around line 13-17: The pre-commit hook using the "go-fmt" entry currently
calls "gofmt -w" directly (id: go-fmt, entry: gofmt -w), which conflicts with
CLAUDE.md guidance to "NEVER use gci/gofmt directly"; change the hook to invoke
the repository formatter target instead (call "make fmt") or otherwise
document/justify this exception: replace the entry value "gofmt -w" with "make
fmt" (or an equivalent wrapper target) so formatting runs via the canonical
Makefile target, ensuring consistency with CLAUDE.md.
In `@CLAUDE.md`:
- Line 30: The README claims the make target "check" runs "all quality gates
(lint + test)" but the Makefile's check target only depends on lint; either
update the docs or change the Makefile. Fix option: modify the Makefile so the
check target depends on both lint and test by changing the check rule to include
the test dependency (ensure targets named "check", "lint", and "test" are used
exactly as in the repo). Alternative: update CLAUDE.md to accurately describe
that "make check" only runs lint. Implement one of these two consistent changes
so docs and the Makefile match.
In `@Makefile`:
- Line 91: Add the generated coverage profile filename to .gitignore: update the
repository's .gitignore to include "coverage.out" so the file produced by the
Makefile test rule (the line running "go test -coverprofile=coverage.out ./...")
is ignored and not accidentally committed.
---
Nitpick comments:
In `@OWNERS`:
- Around line 1-4: The OWNERS file currently lists the same person under both
"reviewers" and "approvers" which creates a single-person bottleneck; update the
YAML by adding at least one additional independent entry to "approvers" (and
optionally "reviewers") so that "ugiordan" is not the sole approver—modify the
"approvers:" list to include another team member's GitHub handle (or a team
alias) alongside "ugiordan" to enforce independent approval.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 7feb173b-5419-4055-89a4-eb7c8d15a639
📒 Files selected for processing (7)
.github/ISSUE_TEMPLATE/bug_report.yml.github/pull_request_template.md.pre-commit-config.yamlCLAUDE.mdMakefileOWNERSdocs/adr/0001-cli-architecture.md
| .PHONY: test | ||
| test: | ||
| go test ./... | ||
| go test -coverprofile=coverage.out ./... |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if coverage.out is in .gitignore
rg -l "coverage.out" .gitignore || echo "coverage.out not found in .gitignore"Repository: opendatahub-io/odh-cli
Length of output: 102
Add coverage.out to .gitignore.
Generated coverage profiles should not be committed to the repository. coverage.out is not currently in .gitignore and will be created by the test target; add it to prevent accidental commits of generated artifacts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` at line 91, Add the generated coverage profile filename to
.gitignore: update the repository's .gitignore to include "coverage.out" so the
file produced by the Makefile test rule (the line running "go test
-coverprofile=coverage.out ./...") is ignored and not accidentally committed.
There was a problem hiding this comment.
Already covered — the existing .gitignore has *.out and coverage.* patterns which match coverage.out. No change needed.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the .gitignore patterns mentioned by the user
cat .gitignore
Length of output: 740
@ugiordan, you're correct — confirmed. The .gitignore already contains both *.out and coverage.* patterns, either of which independently covers coverage.out. My original comment was a false positive; no change is needed.
84c9a8a to
0441744
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Around line 11-16: The fenced code block in CLAUDE.md currently lacks a
language identifier (triggering MD040); update the opening triple-backtick to
include a language tag (e.g., "text") for the block that lists cmd/, pkg/,
internal/, docs/ so the linter recognizes the code fence and silences MD040—edit
the fenced block surrounding those lines in CLAUDE.md to add the language
identifier.
- Around line 35-50: The document contains a contradictory policy: the
"CRITICAL: Always use `make` commands. Never invoke tools directly." rule
conflicts with the test guidance "For test failures: `go test -v ./pkg/<package>
-run TestName`"; pick one consistent policy and update both places
accordingly—either (A) enforce make-only by replacing the `go test` instruction
with a `make` equivalent (e.g., "Use make test for package tests, e.g., make
test PKG=./pkg/<package> RUN=TestName") and add an explicit exception format for
local debugging, or (B) remove/soften the CRITICAL prohibition and allow direct
`go test` by changing the CRITICAL line to permit direct tool invocation for
tests with a recommended `make` wrapper; apply the chosen change to the
"CRITICAL..." header and the "For test failures..." line so both reflect the
same policy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e21d56e1-389b-4d80-beaa-27d00d1e96c3
📒 Files selected for processing (7)
.github/ISSUE_TEMPLATE/bug_report.yml.github/pull_request_template.md.pre-commit-config.yamlCLAUDE.mdMakefileOWNERSdocs/adr/0001-cli-architecture.md
✅ Files skipped from review due to trivial changes (5)
- OWNERS
- .github/pull_request_template.md
- .pre-commit-config.yaml
- docs/adr/0001-cli-architecture.md
- .github/ISSUE_TEMPLATE/bug_report.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
0441744 to
8d644bc
Compare
cd5b917 to
98d2c00
Compare
|
Removed the bug report template. The issues should be reported through Jira since nobody is triaging GitHub issues. Thanks for the feedback @zdtsw. |
6c98dcd to
51055d5
Compare
Summary
Improve AI bug automation readiness score for the odh-cli repository:
.github/ISSUE_TEMPLATE/bug_report.yml) with PII warning and duplicate-check checkbox.github/pull_request_template.md).pre-commit-config.yaml) with golangci-lint and local go hooksdocs/adr/0001-cli-architecture.md)-coverprofile=coverage.out)Test plan
Summary by CodeRabbit
Documentation
Chores