-
Notifications
You must be signed in to change notification settings - Fork 0
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
remove validateFriends #1
Conversation
add smartUnmarshal as helper in fetch tests
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.
@@ -315,7 +276,12 @@ func (s *SurrealDBTestSuite) TestFetch() { | |||
s.NoError(err) | |||
s.NotEmpty(res) | |||
|
|||
s.validateFriends(res, userSlice[0].Friends[0]) | |||
userSlice, err := marshal.SmartUnmarshal[testUserWithFriend[testUserWithFriend[interface{}]]](res, err) |
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.
So this actually isnt deserialising the entire response into nested structs, only pointing to nested structs and deserialising there?
It's really cool that this works. I am wondering if it will be confusing though. From a user perspective, people just want to get their data with a single command. This requires picking each nested doc, and getting the response to populate the nested values.
It's certainly worth having the fetch example.
What do you think we should do? Do you think it's possible to reach the above simplicity without pointing at nested values explicitly?
Also noting, this will likely change with Row and Column
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.
So this actually isnt deserialising the entire response into nested structs, only pointing to nested structs and deserialising there?
It will create and allocate the generic inside the func and unMarshal response array by iterating one by one.
In this case, testUserWithFriend is nested struct.
testUserWithFriend[interface{}] this for
Arhurts friend is John and we want to validate that but we don't need type safety for his John friend.
It's really cool that this works. I am wondering if it will be confusing though. From a user perspective, people just want to get their data with a single command. This requires picking each nested doc, and getting the response to populate the nested values.
I understand why it looks confusing if we go with a simpler user will need to handle this and it will make it more confusing in the end I think this is good for now but I also agree maybe we should make it simpler for future people to understand.
What do you think we should do? Do you think it's possible to reach the above simplicity without pointing at nested values explicitly?
I'm not sure, we could pull off maybe 8-10 lines but it will also make it more complex because we are using raw query we also check errors etc.
Also noting, this will likely change with Row and Column
Yeah, but until that I think a fetch test case will be good to have :)
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.
Yeah, fair point about being able to fetch. Will merge, as it gives us context about how this is going to be currently handled. cc @timpratim about this way of handling fetch going into API
add smartUnmarshal as helper in fetch tests