Skip to content
This repository has been archived by the owner on Dec 23, 2024. It is now read-only.

Convert to using reflect.Value instead of Interface{} #8

Merged
merged 8 commits into from
Jun 26, 2017
Merged

Conversation

talpert
Copy link
Contributor

@talpert talpert commented Jun 26, 2017

reflect.Value has more flexibility than an interface. Change how things work internally, so that is what gets passed around.
This also allows for generic merge functions of maps of any types.
Also switch to requiring the target passed as a pointer and setting its value instead of returning the merged item.

@codecov
Copy link

codecov bot commented Jun 26, 2017

Codecov Report

Merging #8 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master     #8   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           2      2           
  Lines         109    120   +11     
=====================================
+ Hits          109    120   +11
Impacted Files Coverage Δ
merge.go 100% <100%> (ø) ⬆️
mfunc.go 100% <100%> (ø) ⬆️

1 similar comment
@codecov
Copy link

codecov bot commented Jun 26, 2017

Codecov Report

Merging #8 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master     #8   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           2      2           
  Lines         109    120   +11     
=====================================
+ Hits          109    120   +11
Impacted Files Coverage Δ
merge.go 100% <100%> (ø) ⬆️
mfunc.go 100% <100%> (ø) ⬆️

Copy link
Contributor

@caledhwa caledhwa left a comment

Choose a reason for hiding this comment

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

A few small things (comments).

//make a copy here so if there is an error mid way, the target stays in tact
cp := vT.Elem()

//TODO reflect.Indirect(vS)?
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO? Do you wanna get rid of this?

merge_test.go Outdated
newMap, ok := merged.(map[string]interface{})
Expect(ok).To(BeTrue())
Expect(err).ToNot(HaveOccurred())
//newMap, ok := merged.(map[string]interface{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Commented code?

@talpert talpert merged commit 54f1632 into master Jun 26, 2017
@talpert talpert deleted the values branch June 26, 2017 22:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants