Extend duration parser. #34
Conversation
There was a problem hiding this comment.
@timoreimann Wow, great PR :)
Could you add a test with minutes? All tests are using seconds for now. Other than that, looks good!
timoreimann
force-pushed
the
extend-duration-parser
branch
from
a2b9c8b
to
9dc2076
Compare
|
@emilevauge updated PTAL. |
|
@timoreimann tests are failing :'( |
timoreimann
force-pushed
the
extend-duration-parser
branch
from
9dc2076
to
be6d4ba
Compare
|
@emilevauge We're still using Go 1.6 on the project, sub-tests were only introduced in Go 1.7. I bumped the version to 1.8 since we're using that for Traefik as well. Okay with you? |
|
@timoreimann OK on bumping GO to 1.8. |
| @@ -84,12 +84,12 @@ func TestGetTypesRecursive(t *testing.T) { | |||
| config := newConfiguration() | |||
| flagmap := make(map[string]reflect.StructField) | |||
| if err := getTypesRecursive(reflect.ValueOf(config), flagmap, ""); err != nil { | |||
| t.Errorf("Error %s", err.Error()) | |||
| t.Fatalf("Error %s", err.Error()) | |||
There was a problem hiding this comment.
Why a Fatalf is better than just an Errorf ?
There was a problem hiding this comment.
It's more appropriate in this case because the remainder of the test uses the content of flagmap that getTypesRecursive populates. If that function fails, there's no reason we should continue (because we'd be throwing lots of additional test errors that are unrelated to the original cause, which is the failing call in line 86).
timoreimann
force-pushed
the
extend-duration-parser
branch
from
be6d4ba
to
509621e
Compare
Brings support for sub-tests.
- Support TOML unmarshalling. - Default to seconds if no unit specifier is given.
|
@emilevauge test green now. :-) can you merge? I'm missing necessary privileges. |
A working example of the improved parser can be seen in traefik/traefik#1350.
Refs traefik/traefik#1342