Repository navigation
OSAC-2136: trigger design review on README.md and case-insensitive filenames - #101
Conversation
…lenames The enhancement template tells authors to name design docs README.md, but the EP Review Action only triggered on lowercase design.md. Now handles Design.md, DESIGN.md, and README.md inside enhancements/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@ItzikEzra-rh: This pull request references OSAC-2136 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughModified ChangesEP Document Detection Expansion
Estimated code review effort: 1 (Trivial) | ~5 minutes Related Issues: Not specified in the provided context. Related PRs: Not specified in the provided context. Suggested labels: ci, documentation Suggested reviewers: Not specified in the provided context. Poem A rabbit hopped through paths anew, 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/scripts/ep_review.py:
- Around line 45-50: The design-review detection in ep_review.py is using a
broad substring check for "enhancements/" that can match unrelated paths; update
the has_design logic to anchor the check to a real path segment instead of any
substring so only files actually under an enhancements directory qualify. Use
the existing files iteration in the has_design expression and adjust the path
test on f.lower() accordingly, keeping the design.md/readme.md detection
behavior otherwise unchanged.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 52d4ca2c-7eae-4c3c-8837-9763b0f91bcf
📒 Files selected for processing (2)
.github/scripts/ep_review.py.github/workflows/ep-review.yml
| has_prd = any(f.lower().endswith("prd.md") for f in files) | ||
| has_design = any( | ||
| f.lower().endswith("design.md") or | ||
| (f.lower().endswith("readme.md") and "enhancements/" in f.lower()) | ||
| for f in files | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
Substring match on "enhancements/" can false-positive.
"enhancements/" in f.lower() matches any path containing that substring, not just files under an enhancements/ directory — e.g. foo-enhancements/README.md would incorrectly trigger design review, since "foo-enhancements/" contains "enhancements/" as a substring.
♻️ Anchor to path segment instead of substring
has_design = any(
f.lower().endswith("design.md") or
- (f.lower().endswith("readme.md") and "enhancements/" in f.lower())
+ (f.lower().endswith("readme.md") and (
+ f.lower().startswith("enhancements/") or "/enhancements/" in f.lower()
+ ))
for f in files
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| has_prd = any(f.lower().endswith("prd.md") for f in files) | |
| has_design = any( | |
| f.lower().endswith("design.md") or | |
| (f.lower().endswith("readme.md") and "enhancements/" in f.lower()) | |
| for f in files | |
| ) | |
| has_prd = any(f.lower().endswith("prd.md") for f in files) | |
| has_design = any( | |
| f.lower().endswith("design.md") or | |
| (f.lower().endswith("readme.md") and ( | |
| f.lower().startswith("enhancements/") or "/enhancements/" in f.lower() | |
| )) | |
| for f in files | |
| ) |
🤖 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/scripts/ep_review.py around lines 45 - 50, The design-review
detection in ep_review.py is using a broad substring check for "enhancements/"
that can match unrelated paths; update the has_design logic to anchor the check
to a real path segment instead of any substring so only files actually under an
enhancements directory qualify. Use the existing files iteration in the
has_design expression and adjust the path test on f.lower() accordingly, keeping
the design.md/readme.md detection behavior otherwise unchanged.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eranco74, ItzikEzra-rh 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 |
3 participants
Summary
README.md, but the EP Review Action only triggered on lowercasedesign.mdREADME.md— all invisible to the review systemDesign.md,DESIGN.md, andREADME.mdinsideenhancements/directoriesdetect_skillscase-insensitive forprd.mdanddesign.mdTest plan
README.mdchange triggered design review, posted## AI Design Review:comment with design scoresenhancements/scoping prevents triggering on root README.mdTest evidence
ItzikEzra-rh#10
Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes