fix: Handle MaaS acronym in class-generator filename generation#2665
fix: Handle MaaS acronym in class-generator filename generation#2665
Conversation
|
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 selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds preprocessing to treat the "MaaS" acronym as a single word during CamelCase→snake_case conversion and adds runtime validation in the class generator to detect invalid acronym-derived patterns in formatted filenames. Three tests were added to cover MaaS filename outputs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
|
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.
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 `@class_generator/core/generator.py`:
- Around line 61-68: The validation branch that currently calls sys.exit(1)
should instead raise a RuntimeError so callers and the CLI exception handler can
handle it; in the block that logs the invalid filename (around the code in
generate_resource_file_from_dict that checks re.search(r"_[a-z]_",
formatted_kind_str)), keep the LOGGER.error call but replace the sys.exit(1)
with raise RuntimeError("<same or similar descriptive message>") so the error
message is propagated as an exception (consistent with how
class_generator()/generate_resource_file_from_dict() handles other validation
failures).
In `@tests/test_camelcase_to_snake.py`:
- Around line 35-49: Update the three pytest.param expectations to match issue
`#2664`'s filename contract: for inputs "MaaSModelRef", "MaaSAuthPolicy", and
"MaaSSubscription" change the expected outputs from "maas_model_ref",
"maas_auth_policy", "maas_subscription" to "maasmodelref", "maasauthpolicy",
"maassubscription" respectively so the tests assert all-lowercase concatenated
filenames instead of snake_case with underscores.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c84e6e66-3f6f-4384-949d-f9374d02b337
📒 Files selected for processing (3)
class_generator/core/generator.pyocp_resources/utils/utils.pytests/test_camelcase_to_snake.py
|
/approve |
|
/verified |
…atQE#2665) * fix: Handle MaaS acronym in class-generator filename generation Fixes RedHatQE#2664 * fix: Replace sys.exit with raise ValueError for filename validation
Fixes #2664
Changes
normalize_acronymspreprocessing inconvert_camel_case_to_snake_case()to handle mixed-case acronyms likeMaaSthat get incorrectly split tomaa_s_x_) in generated filenames and logs an error directing the user to add the acronym tonormalize_acronyms--dry-runmodeMaaSModelRef,MaaSAuthPolicy,MaaSSubscriptionSummary by CodeRabbit
Bug Fixes
Chores
Tests