-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
Allow named structs to be embedded #303
Conversation
Codecov Report
@@ Coverage Diff @@
## master #303 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 35 35
Lines 2879 2884 +5
=========================================
+ Hits 2879 2884 +5
Continue to review full report at Codecov.
|
@@ -357,6 +357,21 @@ func fieldName(sf reflect.StructField) (string, bool) { | |||
return snaker.CamelToSnake(sf.Name), false | |||
} | |||
|
|||
func isEmbedded(sf reflect.StructField) bool { | |||
// anonymous structs are always embedded |
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.
Is this still true to provide backwards compatibility?
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.
anonymous struct is basically an embedded struct, so I think no problem
@@ -357,6 +357,21 @@ func fieldName(sf reflect.StructField) (string, bool) { | |||
return snaker.CamelToSnake(sf.Name), false | |||
} | |||
|
|||
func isEmbedded(sf reflect.StructField) bool { | |||
// anonymous structs are always embedded |
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.
anonymous struct is basically an embedded struct, so I think no problem
document_meta.go
Outdated
} | ||
// search for embedded tag | ||
tags := strings.Split(sf.Tag.Get("db"), ",") | ||
for i, tag := range tags { |
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.
other codes uses strings.HasSuffix
(example)
I think the advantage of using HasSuffix is it doesn't need intermediary slice
but this is fine as well because it'll be cached anyway
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.
If HasSuffix is ok, let's use it.
Just feels a bit weird as this means that we can't have more than one struct tag (except for the 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 👾
Follow up on #262 that fixes #277 and #296