-
Notifications
You must be signed in to change notification settings - Fork 136
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
don't modify input when converting to orderedmap #870
don't modify input when converting to orderedmap #870
Conversation
"github.com/vmware-tanzu/carvel-ytt/pkg/orderedmap" | ||
) | ||
|
||
func TestFromUnorderedMaps(t *testing.T) { |
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.
Test fails without the changes to convert.go
|
||
// some sources (e.g. toml library) yield specific type of slices. | ||
// slices in Go are not covariant, so each flavor of slice must have its own case, here. | ||
// process these exactly the same way we do for generic slices (prior case) | ||
case []map[string]interface{}: | ||
resultArray := make([]interface{}, len(typedObj)) | ||
result := make([]interface{}, len(typedObj)) |
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.
Renamed this because all the other case blocks use result
as the new variable name
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.
LGTM
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.
1 nit, other part looks fine.
@@ -0,0 +1,23 @@ | |||
package orderedmap_test | |||
|
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.
nit: could you please add header.
@notoriaga I think commit should be rebased because we recently changes the module name to |
1322d92
to
6b3bdc8
Compare
Signed-off-by: notoriaga <[email protected]>
Signed-off-by: notoriaga <[email protected]>
6b3bdc8
to
1b92636
Compare
Ahh I see, thanks! |
Signed-off-by: notoriaga <[email protected]>
LGTM |
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.
LGTM
I'm integrating with ytt as a library and noticed that when I convert an object to an
orderedmap.Map
the input object gets modified. I'm not sure if that's intended but right now it requires me to deepcopy the input before converting which is a little unfortunate.