-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[component] make Type implement MarshalText #9856
[component] make Type implement MarshalText #9856
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9856 +/- ##
==========================================
- Coverage 91.13% 91.12% -0.01%
==========================================
Files 353 353
Lines 18746 18748 +2
==========================================
Hits 17085 17085
- Misses 1333 1335 +2
Partials 328 328 ☔ View full report in Codecov by Sentry. |
Also, the test for this functionality is pretty useless. It'd be great to update it to compare the expected output with an actual yaml string instead of doing the same marshaling. Could you please update it? |
64b6476
to
ee799d9
Compare
ee799d9
to
9f825ac
Compare
I believe the windows test failure is unrelated |
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.
Thanks, @TylerHelmuth!
Description:
Adds
MarshalYAML
function so thatType
can be properly marshaled as yaml.We could alternatively fix this issue by updating the
components
command to explicitly callType.String
and use that value instead ofcomponent.Type
in its struct.Link to tracking Issue:
Fixes #9855
Testing:
Tested locally using
otelcorecol
. The unit testTestNewBuildSubCommand
didnt catch the issue because it was also usingyaml.Marshal
like the command, so the expected value also had{}
as the name.