-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add new report to track properties that get overwritten during flattening #3013
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3013 +/- ##
==========================================
+ Coverage 60.37% 60.41% +0.03%
==========================================
Files 65 65
Lines 10897 10912 +15
==========================================
+ Hits 6579 6592 +13
- Misses 3775 3777 +2
Partials 543 543 ☔ View full report in Codecov by Sentry. |
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.
Core change looks good. A couple of minor nits inline.
Also, can we add a brief explanation of the report to reports/README.md
?
134949e
to
e535245
Compare
Skip the flattening of nested properties indicated by [x-ms-client-flatten](https://github.com/Azure/autorest/blob/main/docs/extensions/readme.md#x-ms-client-flatten) if it would lead to overwriting a property, creating incorrect schema and SDKs. This case happens when inner and outer property have the same name. For a report on all occurrences see #3013. This change is breaking and could therefore only be applied to v3 of the provider. The PR is written to be reviewed commit by commit. It supersedes the previous #3801. **I recommend hiding whitespace when reviewing since the level of indentation of otherwise unaffected code changed.** Resolves #3195
The Azure spec has an annotation x-ms-client-flatten which instructs the schema generator to flatten the property into the parent object. However, there are cases where this leads to overwriting a property, creating incorrect schema and SDKs, when inner and outer property have the same name. I discovered this problem accidentally. First step is to quantify, which this report is for.