Avoid touching the original structure of the options #523
Conversation
rubenvp8510
force-pushed
the
Issue-304
branch
3 times, most recently
from
c454cfd
to
67d7d88
Compare
Codecov Report
@@ Coverage Diff @@
## master #523 +/- ##
=========================================
- Coverage 91.96% 91.8% -0.17%
=========================================
Files 65 65
Lines 3300 3307 +7
=========================================
+ Hits 3035 3036 +1
- Misses 185 188 +3
- Partials 80 83 +3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #523 +/- ##
==========================================
+ Coverage 91.96% 91.97% +<.01%
==========================================
Files 65 65
Lines 3300 3304 +4
==========================================
+ Hits 3035 3039 +4
Misses 185 185
Partials 80 80
Continue to review full report at Codecov.
|
Signed-off-by: Ruben Vargas <ruben.vp8510@gmail.com>
rubenvp8510
changed the title
| o.json = json | ||
| bytes, err := o.MarshalJSON() | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, bytes, json) |
There was a problem hiding this comment.
What is the behavior before this change? {"cassandra.servers": "cassandra:9042"}?
There was a problem hiding this comment.
The change this PR does is on marshal and unmarshall, previously it flats all options into condensed form, now I don't do that, because it could produce Jaeger resources with mixed condensed and structured form of options, I just leave the options intact.
So for this case will return the JSON in the way it is, this is: {"cassandra": {"servers": "cassandra:9042"}} , but from code perspective you can still access to a map with condensed form of properties.
I did in this way trying to preserve backwards compatibility, if someone is already using condensed form, it will work, same for users using the structured form of options.
Other option is to change this to map[string]string, but that breaks the compatibility for users that uses structured form
Signed-off-by: Ruben Vargas ruben.vp8510@gmail.com
Fixes Issue #304
Changed the way that the options is serialized/deserialized, now options does not touch the yaml structure on an update.
This is similar to Freeform type, but it has internally a method for "flat" options, so this support both types of properties, condensed form and normal structure, but not a mix of them.
may be we can/should merge this type with Freeform ?