-
Notifications
You must be signed in to change notification settings - Fork 139
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
Make CompositeType.Members field an ordered map #581
Conversation
d56c16f
to
977b1a1
Compare
977b1a1
to
a1bfd4e
Compare
465709d
to
be9dbdb
Compare
@SupunS can you please rebase on |
@turbolent Rebased to master. |
runtime/convertTypes.go
Outdated
member, ok := t.Members.Get(identifier) | ||
|
||
if member.IgnoreInSerialization { | ||
if !ok || member.IgnoreInSerialization { |
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 there ever a case where the lookup would fail? Is it rather an implementation error when the lookup fails?
Same question below.
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.
AFAIU Fields
are a subset of Members
. So the lookup should always succeed.
Maybe a panic would be a good idea.
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.
Nice! Just a couple questions / suggestions
testFunctionMember := structureType.Members["test"] | ||
|
||
testFunctionMember, ok := structureType.Members.Get("test") | ||
require.True(t, ok) |
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.
👍
@SupunS thanks for rebasing, looks good |
1b4def8
to
d80fc38
Compare
d80fc38
to
f287790
Compare
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.
Nice!
Description
Work towards #436
string
keys and*Member
valuesCompositeType.Members
field to the generated ordered map.For contributor use:
master
branchFiles changed
in the Github PR explorer