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

Support time.Time #100

Merged
merged 5 commits into from
Aug 2, 2022
Merged

Support time.Time #100

merged 5 commits into from
Aug 2, 2022

Conversation

mcdee
Copy link
Contributor

@mcdee mcdee commented Aug 2, 2022

This allows encoding and decoding of the time.Time type in structs, to uint64 values.

Copy link
Owner

@ferranbt ferranbt left a comment

Choose a reason for hiding this comment

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

It looks good. Only minor changes to follow the same standard for the generated code.

tests/time.go Outdated Show resolved Hide resolved
tests/time_encoding.go Outdated Show resolved Hide resolved
@@ -104,6 +104,9 @@ func (v *Value) unmarshal(dst string) string {
case TypeBool:
return fmt.Sprintf("::.%s = ssz.UnmarshalBool(%s)", v.name, dst)

case TypeTime:
return fmt.Sprintf("::.%s = time.Unix(int64(ssz.UnmarshallUint64(%s)), 0).UTC()", v.name, dst)
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of doing a complex generated code, add an entry at the root of the project with a helper function:

function UnmarshalTime(buf []byte) time.Time {
    time.Unix(int64(UnmarshallUint64(buf)), 0).UTC()
}

and use that instead on the code.

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry, you do not need to do this after this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be addressed, but please take a look to confirm.

(Note that this removes the requirement for the time import in the earlier comment).

encode.go Outdated Show resolved Hide resolved
encode.go Outdated Show resolved Hide resolved
sszgen/testcases/time.go Show resolved Hide resolved
@ferranbt ferranbt merged commit 82e4cce into ferranbt:master Aug 2, 2022
@henridf henridf mentioned this pull request Sep 9, 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.

2 participants