-
Notifications
You must be signed in to change notification settings - Fork 118
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
types: add custom Duration type #342
Conversation
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.
@alessandro-sorint Thanks for the PR.
It should be better to put these files under services/types/duration[_test].go
I also think that using a struct with an embedded time.Duration could be cleaner.
35b905e
to
a5ecd43
Compare
a5ecd43
to
f906304
Compare
services/types/duration_test.go
Outdated
{ | ||
name: "test duration string", | ||
expected: Duration{10 * time.Second}, | ||
jsonDuration: ` |
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.
Just use one line to save some space.
services/types/duration_test.go
Outdated
for _, tt := range tests { | ||
result, err := json.Marshal(tt.duration) | ||
if err != nil { | ||
t.Fatal("failed to marshal duration: %w", 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.
t.Fatalf and use %v since it's not error wrapping
services/types/duration_test.go
Outdated
for _, tt := range tests { | ||
var result testDurationType | ||
if err := json.Unmarshal([]byte(tt.jsonDuration), &result); err != nil { | ||
t.Fatal("failed to unmarshal json: %w", 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.
t.Fatalf and use %v since it's not error wrapping
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.
Please rename the commit to standard conventions: types: add custom Duration type
f906304
to
6e3030a
Compare
6e3030a
to
d59ff9f
Compare
ok I renamed |
@alessandro-sorint Thanks merging. |
Added Duration custom type for unmarshall json duration as human format or nanoseconds