feat: add new global DurationFieldFormat #733
Conversation
|
@rs did a short local test. Could not find a good place to put a unit tests for it but seems to work just fine. |
|
I do think adding test is important and useful. |
- This deprecates DurationFieldInteger in favor of DurationFieldFormat with the value of DurationFormatInt
sruehl
force-pushed
the
feat/duration_strings
branch
from
5636ec0 to
9e95eb5
Compare
|
Sorry for the delay. Could you please fix the conflict so I could merge this? Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new global DurationFieldFormat configuration option to control how duration fields are serialized, deprecating the existing DurationFieldInteger boolean in favor of a more flexible string-based format specification.
Key Changes:
- Added three new duration format constants:
DurationFormatFloat,DurationFormatInt, andDurationFormatString - Updated
AppendDurationandAppendDurationsmethods across JSON and CBOR encoders to accept a format string parameter - Added comprehensive test coverage for the new duration formatting options
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| globals.go | Added new DurationFieldFormat global variable and three duration format constants; deprecated DurationFieldInteger |
| encoder.go | Updated encoder interface signatures to include format string parameter |
| internal/json/time.go | Modified duration encoding functions to support format-based serialization with switch statement |
| internal/json/time_test.go | Added test cases for all duration format options |
| internal/cbor/time.go | Modified CBOR duration encoding functions to support format-based serialization |
| internal/cbor/time_test.go | Added test cases for CBOR duration formatting; fixed scientific notation style |
| event.go | Updated all AppendDuration and AppendDurations calls to pass DurationFieldFormat |
| context.go | Updated context duration methods to use new format parameter |
| array.go | Updated array duration method to use new format parameter |
| fields.go | Updated field list duration handling to use new format parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| unit time.Duration | ||
| format string | ||
| useInt bool | ||
| unused int |
There was a problem hiding this comment.
The parameter name 'unused' is unclear. Consider renaming it to 'precision' or another descriptive name that reflects its actual purpose in the AppendDuration function signature.
| unit time.Duration | ||
| format string | ||
| useInt bool | ||
| unused int |
There was a problem hiding this comment.
The parameter name 'unused' is unclear. Consider renaming it to 'precision' or another descriptive name that reflects its actual purpose in the AppendDuration function signature.
|
|
||
| // DurationFieldInteger renders Dur fields as integer instead of float if | ||
| // set to true. | ||
| // Deprecated: use DurationFieldFormat with DurationFormatInt instead. |
There was a problem hiding this comment.
| // Deprecated: use DurationFieldFormat with DurationFormatInt instead. | |
| // Deprecated: use [DurationFieldFormat] with [DurationFormatInt] instead. |
4 participants
This deprecates DurationFieldInteger in favor of DurationFieldFormat with the value of DurationFormatInt
resolves #709