-
Notifications
You must be signed in to change notification settings - Fork 196
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
Flattening #1548
Flattening #1548
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1548 +/- ##
==========================================
- Coverage 63.26% 62.36% -0.91%
==========================================
Files 184 186 +2
Lines 11926 12256 +330
==========================================
+ Hits 7545 7643 +98
- Misses 3707 3926 +219
- Partials 674 687 +13
Continue to review full report at Codecov.
|
hack/generator/pkg/codegen/testdata/Flattening/FlattenedPropertiesAreFlattened.golden
Show resolved
Hide resolved
hack/generator/pkg/astmodel/armconversion/convert_to_arm_function_builder.go
Outdated
Show resolved
Hide resolved
hack/generator/pkg/astmodel/armconversion/convert_to_arm_function_builder.go
Show resolved
Hide resolved
hack/generator/pkg/astmodel/armconversion/convert_to_arm_function_builder.go
Outdated
Show resolved
Hide resolved
hack/generator/pkg/codegen/testdata/Flattening/FlattenedPropertiesAreFlattened.golden
Show resolved
Hide resolved
This will probably have conflicts with #1556 - we should coordinate. |
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.
I think the core of this came out pretty clean actually, nice!
hack/generator/pkg/astmodel/armconversion/convert_from_arm_function_builder.go
Outdated
Show resolved
Hide resolved
hack/generator/pkg/astmodel/armconversion/convert_from_arm_function_builder.go
Outdated
Show resolved
Hide resolved
hack/generator/pkg/astmodel/armconversion/convert_from_arm_function_builder.go
Show resolved
Hide resolved
hack/generator/pkg/astmodel/armconversion/convert_from_arm_function_builder.go
Outdated
Show resolved
Hide resolved
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.
I'm with Matt, this came out really cleanly! Lots of questions but I think those are just getting my head around what's going on rather than problems.
@@ -142,7 +142,9 @@ func (codeGenContext *CodeGenerationContext) GetAllReachableTypes() Types { | |||
for _, pkgImport := range codeGenContext.packageImports.AsSlice() { | |||
types, found := codeGenContext.GetTypesInPackage(pkgImport.packageReference) | |||
if found { | |||
result.AddTypes(types) | |||
for k, v := range types { |
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.
Presumably because this can have replacements that change the type the definition points to?
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.
It can find duplicates, somehow. I didn't think about it too much 🙂
Co-authored-by: Christian Muirhead <christian.muirhead@microsoft.com>
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Closes #1447
What this PR does / why we need it:
Applies the
x-ms-client-flatten
attribute when it is present in the Swagger.Special notes for your reviewer:
Highly recommend reviewing ignoring whitespace.
How does this PR make you feel:
If applicable: