Skip to content
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

Use tag to determine if to flatten or not an embedded anonymous field #85

Merged
merged 3 commits into from
Sep 6, 2022

Conversation

vladshub
Copy link
Contributor

@vladshub vladshub commented Jul 20, 2022

If we would take for example a kube object Pod it has ObjectMeta as an embedded field but if we render it via json.Marshal we would get a nice metadata field in our json.
However in our OpenAPI definition this field would be missing.

Copy link
Owner

@wI2L wI2L left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I would just like a few changes to the tests. Thanks

@@ -48,6 +48,7 @@ type (
*u
uu *u // ignored, unexported field
q // ignored, embedded field of non-struct type
*Q `json:"data"`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add another field that is an exported field of struct type, but that has no tag? To test that its fiels are then inlined.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, could you keep the tag value to Q to match the current output, where each field is named by its type in the final spec.

@@ -70,6 +70,9 @@
"S": {
"type": "integer",
"format": "int32"
},
"data": {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/data/Q

@vladshub
Copy link
Contributor Author

@wI2L thanks for the review, fixed the issues.

@codecov
Copy link

codecov bot commented Sep 6, 2022

Codecov Report

Merging #85 (410edcd) into master (9301be1) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
+ Coverage   94.34%   94.35%   +0.01%     
==========================================
  Files           7        7              
  Lines         973      975       +2     
==========================================
+ Hits          918      920       +2     
  Misses         39       39              
  Partials       16       16              
Impacted Files Coverage Δ
openapi/generator.go 93.91% <100.00%> (+0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@wI2L wI2L merged commit 36d2229 into wI2L:master Sep 6, 2022
@vladshub
Copy link
Contributor Author

vladshub commented Sep 7, 2022

Thank you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants