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

types: add custom Duration type #342

Merged
merged 1 commit into from
May 31, 2022

Conversation

alessandro-sorint
Copy link
Contributor

Added Duration custom type for unmarshall json duration as human format or nanoseconds

Copy link
Member

@sgotti sgotti left a 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.

internal/services/types/types.go Outdated Show resolved Hide resolved
internal/services/types/types.go Outdated Show resolved Hide resolved
internal/services/types/types_test.go Outdated Show resolved Hide resolved
internal/services/types/types.go Outdated Show resolved Hide resolved
internal/services/types/types.go Outdated Show resolved Hide resolved
internal/services/types/types.go Outdated Show resolved Hide resolved
services/types/duration_test.go Outdated Show resolved Hide resolved
services/types/duration_test.go Outdated Show resolved Hide resolved
{
name: "test duration string",
expected: Duration{10 * time.Second},
jsonDuration: `
Copy link
Member

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.

for _, tt := range tests {
result, err := json.Marshal(tt.duration)
if err != nil {
t.Fatal("failed to marshal duration: %w", err)
Copy link
Member

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

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

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

@sgotti sgotti changed the title Added Duration custom type types: add custom Duration type May 31, 2022
Copy link
Member

@sgotti sgotti left a 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

@alessandro-sorint
Copy link
Contributor Author

Please rename the commit to standard conventions: types: add custom Duration type

ok I renamed

@sgotti
Copy link
Member

sgotti commented May 31, 2022

@alessandro-sorint Thanks merging.

@sgotti sgotti merged commit b8c78cd into agola-io:master May 31, 2022
@alessandro-sorint alessandro-sorint deleted the duration-custom branch June 7, 2022 09:23
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