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

add hash:"string" tags support #11

Merged
merged 2 commits into from
May 11, 2017
Merged

add hash:"string" tags support #11

merged 2 commits into from
May 11, 2017

Conversation

fortytw2
Copy link
Contributor

@fortytw2 fortytw2 commented May 10, 2017

This PR adds support for hashing things that implement fmt.Stringer via an additional struct tag to maintain backwards compatibility. This is especially useful when you have structs that contain time.Time in them, as there are no exported fields in it.

edit: this looks awfully similar in purpose to hashicorp/vault#2689

hashstructure.go Outdated
"fmt"
"hash"
"hash/fnv"
"reflect"
)

var (
ErrNotStringer = errors.New("field tag set to string, but struct does not implement fmt.Stringer")
Copy link
Owner

Choose a reason for hiding this comment

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

I think we should include the field name in the error output otherwise this can be pretty vague.

Copy link
Owner

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

This looks really great. I requested one small change. I'd do it myself but don't have time at the moment. If you get to it I'll merge.

Great job! Thank you!

@fortytw2
Copy link
Contributor Author

Updated! Thanks 💯

@mitchellh mitchellh merged commit 9204ce5 into mitchellh:master May 11, 2017
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