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

Allow named structs to be embedded #303

Merged
merged 3 commits into from Jul 5, 2022

Conversation

dranikpg
Copy link
Contributor

@dranikpg dranikpg commented Jul 4, 2022

Follow up on #262 that fixes #277 and #296

@codecov
Copy link

codecov bot commented Jul 4, 2022

Codecov Report

Merging #303 (2112d28) into master (d417e5f) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #303   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           35        35           
  Lines         2879      2884    +5     
=========================================
+ Hits          2879      2884    +5     
Impacted Files Coverage Δ
document_meta.go 100.00% <100.00%> (ø)

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 d417e5f...2112d28. Read the comment docs.

@@ -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
Copy link
Contributor Author

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?

Copy link
Member

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
Copy link
Member

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 {
Copy link
Member

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

Copy link
Contributor Author

@dranikpg dranikpg Jul 4, 2022

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) 🤔

document_meta.go Outdated Show resolved Hide resolved
Copy link
Member

@Fs02 Fs02 left a comment

Choose a reason for hiding this comment

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

LGTM 👾

@Fs02 Fs02 merged commit 36bf6b0 into go-rel:master Jul 5, 2022
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.

Add support for embedded no anonymous fields
2 participants