Fully overrides default slice fields of config (#4001) #6163
Conversation
Codecov ReportBase: 92.06% // Head: 92.06% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #6163 +/- ##
=======================================
Coverage 92.06% 92.06%
=======================================
Files 219 219
Lines 13242 13252 +10
=======================================
+ Hits 12191 12201 +10
Misses 828 828
Partials 223 223
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| // To address this we zeroes every slice before unmarshalling unless user provided slice is nil. | ||
| func zeroesSliceHookFunc() mapstructure.DecodeHookFuncValue { | ||
| return func(from reflect.Value, to reflect.Value) (interface{}, error) { | ||
| if from.Kind() == reflect.Slice && from.IsNil() { |
There was a problem hiding this comment.
Len() == 0? No need to re-initialize if empty?
There was a problem hiding this comment.
If user provides empty list in the config explicitly then we zeroes the default value of the field.
There was a problem hiding this comment.
@bogdandrutu do you mean if both source and destination are 0 then we should just return?
There was a problem hiding this comment.
Lets start with this:
nil != initialized and len 0
foo:foo: []What is the behavior we want in these cases? Please document them somewhere, in order for me to understand if the implementation is ok.
mcdoker18
force-pushed
the
feature/slice_config
branch
2 times, most recently
from
62b2fba to
ca970c1
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
mcdoker18
force-pushed
the
feature/slice_config
branch
from
ca970c1 to
9213f8d
Compare
|
@bogdandrutu @dmitryax please can you take a look? All comments are fixed. |
mapstructure library doesn't override full slice during unmarshalling. Origin issue: mitchellh/mapstructure#74 (comment) To address this we empty every slice before unmarshalling unless user provided slice is nil.
mcdoker18
force-pushed
the
feature/slice_config
branch
from
9213f8d to
cfebf0b
Compare
|
@dmitryax fixed |
There was a problem hiding this comment.
The PR looks ok to me, will merge once @bogdandrutu's comments have been addressed
| // To address this we zeroes every slice before unmarshalling unless user provided slice is nil. | ||
| func zeroesSliceHookFunc() mapstructure.DecodeHookFuncValue { | ||
| return func(from reflect.Value, to reflect.Value) (interface{}, error) { | ||
| if from.Kind() == reflect.Slice && from.IsNil() { |
There was a problem hiding this comment.
@bogdandrutu do you mean if both source and destination are 0 then we should just return?
| } | ||
|
|
||
| if to.CanSet() && to.Kind() == reflect.Slice { | ||
| to.Set(reflect.MakeSlice(reflect.SliceOf(to.Type().Elem()), 0, 0)) |
There was a problem hiding this comment.
why 0 capacity? I think default in golang is 8?
| // To address this we zeroes every slice before unmarshalling unless user provided slice is nil. | ||
| func zeroesSliceHookFunc() mapstructure.DecodeHookFuncValue { | ||
| return func(from reflect.Value, to reflect.Value) (interface{}, error) { | ||
| if from.Kind() == reflect.Slice && from.IsNil() { |
There was a problem hiding this comment.
Lets start with this:
nil != initialized and len 0
foo:foo: []What is the behavior we want in these cases? Please document them somewhere, in order for me to understand if the implementation is ok.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Description:
mapstructure library doesn't override full slice during unmarshalling. Origin issue: mitchellh/mapstructure#74 (comment) To address this we zeroes every slice before unmarshalling unless user provided slice is nil.
Link to tracking Issue: #4001