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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughA public class in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ocp_resources/utils/utils.py (1)
91-96: LGTM! Clean approach for acronym normalization.The normalization dictionary is a maintainable solution for handling mixed-case acronyms. The inline comment clearly explains the rationale.
Consider adding an example to the docstring (around lines 70-77) to document this behavior:
>>> convert_camel_case_to_snake_case("MaaSModelRef") 'maas_model_ref'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ocp_resources/utils/utils.py` around lines 91 - 96, Add a short doctest/example to the convert_camel_case_to_snake_case function's docstring demonstrating the acronym normalization behavior (e.g., showing "MaaSModelRef" -> "maas_model_ref"); update the docstring near lines ~70-77 to include a one-line example and expected output so maintainers see how normalize_acronyms (and the replacement logic that maps "MaaS" to "Maas") affects results.tests/test_camelcase_to_snake.py (1)
35-49: LGTM! Good test coverage for MaaS acronym normalization.The new test cases properly validate the expected snake_case conversion for mixed-case acronyms. The test IDs are descriptive and follow the existing naming pattern.
Consider adding edge cases for broader coverage:
"MaaS"alone →"maas"(standalone acronym)- Acronym followed by another acronym (if applicable in your domain)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_camelcase_to_snake.py` around lines 35 - 49, Add two additional pytest.param cases to tests/test_camelcase_to_snake.py: one for the standalone acronym "MaaS" expecting "maas" (id="standalone_acronym_maas"), and one for an acronym followed immediately by another acronym (e.g., "MaaSAPI" expecting "maas_api" with id="acronym_followed_by_acronym_maas_api" or a domain-appropriate pair like "MaaSID"->"maas_id"); place them alongside the existing MaaS cases so the test matrix covers the standalone and acronym+acronym edge cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ocp_resources/utils/utils.py`:
- Around line 91-96: Add a short doctest/example to the
convert_camel_case_to_snake_case function's docstring demonstrating the acronym
normalization behavior (e.g., showing "MaaSModelRef" -> "maas_model_ref");
update the docstring near lines ~70-77 to include a one-line example and
expected output so maintainers see how normalize_acronyms (and the replacement
logic that maps "MaaS" to "Maas") affects results.
In `@tests/test_camelcase_to_snake.py`:
- Around line 35-49: Add two additional pytest.param cases to
tests/test_camelcase_to_snake.py: one for the standalone acronym "MaaS"
expecting "maas" (id="standalone_acronym_maas"), and one for an acronym followed
immediately by another acronym (e.g., "MaaSAPI" expecting "maas_api" with
id="acronym_followed_by_acronym_maas_api" or a domain-appropriate pair like
"MaaSID"->"maas_id"); place them alongside the existing MaaS cases so the test
matrix covers the standalone and acronym+acronym edge cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0cb2d479-4b08-4ef0-bb56-6b82e3f1fa19
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
class_generator/core/generator.pyocp_resources/maas_model_ref.pyocp_resources/utils/utils.pypyproject.tomltests/test_camelcase_to_snake.py
✅ Files skipped from review due to trivial changes (1)
- pyproject.toml
Short description:
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
Bug:
Summary by CodeRabbit