Skip to content
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

set default values of fields of a pointer type field #18

Merged
merged 5 commits into from
Jun 9, 2020

Conversation

sdghchj
Copy link
Contributor

@sdghchj sdghchj commented Jun 4, 2020

fix #16 #17

@sdghchj
Copy link
Contributor Author

sdghchj commented Jun 4, 2020

And add go.mod

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2020

Codecov Report

Merging #18 into master will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
- Coverage   98.29%   98.26%   -0.03%     
==========================================
  Files           2        2              
  Lines         117      115       -2     
==========================================
- Hits          115      113       -2     
  Misses          1        1              
  Partials        1        1              
Impacted Files Coverage Δ
defaults.go 98.21% <100.00%> (-0.04%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c58e679...39b28d3. Read the comment docs.

Copy link
Owner

@creasty creasty left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

if defaultVal != "" && defaultVal != "{}" {
if err := json.Unmarshal([]byte(defaultVal), ref.Interface()); err != nil {
if err := json.Unmarshal([]byte(defaultVal), field.Addr().Interface()); err != nil {
Copy link
Owner

Choose a reason for hiding this comment

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

Neat 👍

defaults.go Outdated
return err
}
callSetter(ref.Interface())
field.Set(ref.Elem())
callSetter(field.Addr().Interface())
Copy link
Owner

Choose a reason for hiding this comment

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

It seems to be a redundant call as you're already doing it in Set.
Either one of them should be removed. Perhaps the latter.

@creasty
Copy link
Owner

creasty commented Jun 9, 2020

Could you also change the title to be more descriptive?

@sdghchj sdghchj changed the title fix issues set default values of fields of a pointer type field Jun 9, 2020
@sdghchj
Copy link
Contributor Author

sdghchj commented Jun 9, 2020

@creasty review again please

Copy link
Owner

@creasty creasty left a comment

Choose a reason for hiding this comment

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

Looking good. Thanks!

@creasty creasty merged commit 0dea786 into creasty:master Jun 9, 2020
@creasty
Copy link
Owner

creasty commented Jun 9, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The Setter interface only works on struct pointer fields, not the struct itself
3 participants