-
-
Notifications
You must be signed in to change notification settings - Fork 424
Add comprehensive feature metadata to schema version dataclasses #2946
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 13 seconds before requesting another review. ⌛ 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. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughNew boolean feature flags added to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
|
📚 Docs Preview: https://pr-2946.datamodel-code-generator.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/parser/test_schema_version.py (1)
125-138: Consider explicit new fields in Draft 6+ tests for completeness.The Draft 4 test explicitly includes
const_support=Falseandproperty_names=False, but Draft 6+ tests rely on defaults (True). While functionally correct (inline_snapshot matches the full dataclass), explicitly including these fields in the other tests would improve test clarity and make the version differences more apparent.Similarly,
test_openapi_features_v30(lines 212-227) doesn't explicitly includewebhooks=Falseandref_sibling_keywords=False.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/datamodel_code_generator/parser/schema_version.pytests/parser/test_schema_version.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Analyze (python)
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.11 on macOS
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.10 on macOS
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.13 on macOS
- GitHub Check: 3.11 on Windows
- GitHub Check: benchmarks
🔇 Additional comments (9)
tests/parser/test_schema_version.py (3)
107-122: LGTM!The test correctly verifies that Draft 4 lacks
constandpropertyNamessupport (both introduced in Draft 6), matching the implementation.
230-247: LGTM!The test correctly verifies that OpenAPI 3.1 has
webhooksandref_sibling_keywordssupport, matching the implementation.
250-267: LGTM!The test correctly verifies that OpenAPI Auto defaults to latest features, including
webhooksandref_sibling_keywords.src/datamodel_code_generator/parser/schema_version.py (6)
121-157: LGTM!The new feature flags for
const_support,property_names,contains, anddeprecated_keywordhave appropriate defaults and accurate metadata reflecting their introduction versions and implementation status.
158-239: LGTM!The unsupported feature flags are well-documented with accurate metadata. All have
default=Falseandstatus="not_supported", which is correct for features not yet implemented.
244-257: LGTM!Draft 4 correctly sets
const_support=Falseandproperty_names=False, as these features were introduced in Draft 6.
258-301: Verify: Shoulddeprecated_keywordbe set toTruefor Draft 2019-09+?The
deprecated_keywordfield hasstatus="partial"andintroduced="2019-09". Currently, Draft 2019-09 and later versions don't explicitly set this field, relying on the default ofFalse.If the intent is to track feature availability (even if partially implemented), consider setting
deprecated_keyword=Truefor Draft 2019-09+. If the intent is to track full implementation support, the current behavior is correct.
332-396: LGTM!The new OpenAPI feature flags have appropriate defaults and accurate metadata.
webhooksandref_sibling_keywordscorrectly default toFalseand are explicitly enabled for OpenAPI 3.1+.
415-429: LGTM!The non-V30 case correctly enables
webhooks=Trueandref_sibling_keywords=True, as these are OpenAPI 3.1 features.
CodSpeed Performance ReportMerging this PR will not alter performanceComparing
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2946 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 94 94
Lines 17696 17716 +20
Branches 2037 2037
=========================================
+ Hits 17696 17716 +20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🤖 Generated by GitHub Actions
Generated by GitHub Actions
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.