Encode and decode time.Duration fields (#201) #246
Conversation
This uses `time.ParseDuration` and `time.Duration.String`, and is compatible with gopkg.in/yaml's encoding.
Codecov Report
@@ Coverage Diff @@
## master #246 +/- ##
==========================================
+ Coverage 78.47% 78.73% +0.25%
==========================================
Files 12 12
Lines 3419 3451 +32
==========================================
+ Hits 2683 2717 +34
+ Misses 502 501 -1
+ Partials 234 233 -1 |
decode.go
Outdated
| func (d *Decoder) decodeDuration(ctx context.Context, dst reflect.Value, src ast.Node) error { | ||
| t, err := d.castToDuration(src) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
| return err | |
| return errors.Wrapf(err, "failed to convert to duration") |
There was a problem hiding this comment.
This is already wrapped (if necessary) inside castToDuration
There was a problem hiding this comment.
Yes! That's right. However, we still need to wrap the err in order to get the stack trace printed correctly.
There was a problem hiding this comment.
Hm, are you saying I should wrap it twice? I'm currently matching the existing implementation of decodeTime, which also doesn't wrap the error a second time.
There was a problem hiding this comment.
Yes, it's need to improve debuggability ( please try to output error with %+v )
Oh, decodeTime's case is wrong. It's need to wrap...
There was a problem hiding this comment.
I added the call to Wrapf on both decodeDuration and decodeTime. I also added a test case so you can see the resulting messages. It looks like in most cases the stack trace is getting lost anyway at line 1059 when the error is unwrapped before being returned.
|
The latest test has failed. Probably this issue will be fixed by merging the master branch for tracking upstream. |
Would you prefer a merge commit or a rebase? |
Either of them is ok 👍 |
|
Sorry for the delay, I merged in the current masster. |
This uses
time.ParseDurationandtime.Duration.String, and iscompatible with gopkg.in/yaml's encoding.