Conversation
…ging flags - Accept LICENSE, LICENSE.md, LICENSE.txt as valid license files; flag non-canonical names with rename suggestion - Accept README.md, README.txt, README as valid readme files; flag non-canonical names with rename suggestion - Detect pyproject.toml and flag setup.py, requirements.txt, MANIFEST.in as legacy (🗑️, no CI failure) - Fix duplicate setup.py entry in results when both pyproject.toml and setup.py exist - Remove dead REQUIRED_FILES list and hardcoded setup.py hide logic in formatter Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 39 minutes and 36 seconds. ⌛ 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. 📝 WalkthroughWalkthroughThe changes enhance repository health checking by introducing legacy file handling and canonical filename management. Legacy files now display with a trash icon and optional reason message, while the script identifies canonical filenames (README.md, LICENSE) and suggests renames for alternatives. The workflow icon logic shifts to flag unresolved optional items with warning indicators. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
🧹 Nitpick comments (1)
scripts/check_repo_health.py (1)
59-87: Add regression tests forrename_toandlegacyreport fields.This change adds new output contract fields (
rename_to,legacy,legacy_reason) but current tests intest/test_scripts.pydo not assert them. Please add targeted checks so workflow rendering behavior is locked in.Also applies to: 118-131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check_repo_health.py` around lines 59 - 87, Update the tests in test/test_scripts.py to assert the new report fields added by scripts/check_repo_health.py: verify that readme_entry and license_entry include "rename_to" equal to README_CANONICAL and LICENSE_CANONICAL respectively when a non-canonical file is found, and add assertions for any "legacy" and "legacy_reason" fields where applicable (ensure the tests inspect the results list entries created by readme_entry and license_entry and assert existence/values of "rename_to", "legacy", and "legacy_reason" to lock the output contract); ensure you reference the results entries produced by the script (e.g., entries labeled "README" or "License file") so the assertions remain robust if order changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check_repo_health.py`:
- Around line 105-107: The check currently skips legacy files only when they
exist, so when pyproject.toml is present but setup.py is missing the script
still emits a warning; update the conditional to skip legacy filenames whenever
has_pyproject is true regardless of exists. Modify the block using
has_pyproject, filename, legacy_filenames, and exists so it reads something
like: if has_pyproject and filename in legacy_filenames: continue (i.e., remove
the exists requirement) so optional setup.py is suppressed when pyproject.toml
is present.
---
Nitpick comments:
In `@scripts/check_repo_health.py`:
- Around line 59-87: Update the tests in test/test_scripts.py to assert the new
report fields added by scripts/check_repo_health.py: verify that readme_entry
and license_entry include "rename_to" equal to README_CANONICAL and
LICENSE_CANONICAL respectively when a non-canonical file is found, and add
assertions for any "legacy" and "legacy_reason" fields where applicable (ensure
the tests inspect the results list entries created by readme_entry and
license_entry and assert existence/values of "rename_to", "legacy", and
"legacy_reason" to lock the output contract); ensure you reference the results
entries produced by the script (e.g., entries labeled "README" or "License
file") so the assertions remain robust if order changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93e2f48c-8f6f-4037-84da-b9673be07869
📒 Files selected for processing (2)
.github/workflows/repo-health.ymlscripts/check_repo_health.py
| # skip setup.py here — it will be reported as legacy below | ||
| if has_pyproject and filename in legacy_filenames and exists: | ||
| continue |
There was a problem hiding this comment.
Suppress setup.py warnings when pyproject.toml already exists.
At Line 106, setup.py is skipped only when it exists. If pyproject.toml exists and setup.py is absent, the script still emits a missing optional setup.py entry, which becomes a
Proposed fix
- # skip setup.py here — it will be reported as legacy below
- if has_pyproject and filename in legacy_filenames and exists:
+ # when pyproject.toml exists, do not report setup.py in setup group;
+ # if setup.py exists, it is reported below as legacy
+ if has_pyproject and filename == "setup.py":
continue📝 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.
| # skip setup.py here — it will be reported as legacy below | |
| if has_pyproject and filename in legacy_filenames and exists: | |
| continue | |
| # when pyproject.toml exists, do not report setup.py in setup group; | |
| # if setup.py exists, it is reported below as legacy | |
| if has_pyproject and filename == "setup.py": | |
| continue |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/check_repo_health.py` around lines 105 - 107, The check currently
skips legacy files only when they exist, so when pyproject.toml is present but
setup.py is missing the script still emits a warning; update the conditional to
skip legacy filenames whenever has_pyproject is true regardless of exists.
Modify the block using has_pyproject, filename, legacy_filenames, and exists so
it reads something like: if has_pyproject and filename in legacy_filenames:
continue (i.e., remove the exists requirement) so optional setup.py is
suppressed when pyproject.toml is present.
…M availability check The entry point parsing block (import test + interface check) was gated behind the ovos-plugin-manager import, so it never ran when OPM was absent even though the import tests themselves have no OPM dependency. Moved the block before the OPM import guard so import_ok is always populated when test_import=True. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
LICENSE,LICENSE.md,LICENSE.txtall satisfy the license check. Non-canonical names render as✅ \LICENSE.md` — License file (consider renaming to `LICENSE`)`.README.md,README.txt,READMEall satisfy the readme check. Same rename hint for non-canonical names.pyproject.tomlis present, any co-existingsetup.py,requirements.txt, orMANIFEST.inis flagged as 🗑️ legacy with a note — no CI failure, just advisory.setup.pywas previously added to results twice when bothpyproject.tomlandsetup.pyexisted (once from SETUP_FILES loop, once from LEGACY_IF_PYPROJECT). It's now skipped in the SETUP_FILES loop when it will be reported as legacy. The corresponding hardcodedsetup.pyhide logic in the workflow formatter is removed.REQUIRED_FILES = []variable.Test plan
LICENSE→✅ \LICENSE` — License file`LICENSE.md→✅ \LICENSE.md` — License file (consider renaming to `LICENSE`)`LICENSE.txt→ same rename hint❌ \LICENSE` — License file`pyproject.toml+setup.py+requirements.txt+MANIFEST.in→ all three flagged as 🗑️ legacy, no failuresetup.py(nopyproject.toml) →✅ \setup.py``🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes