feat: duration renderer for SIGN_MODE_TEXTUAL #13312
Conversation
There was a problem hiding this comment.
LGTM overall, thanks @JimLarson. One comment is whether we need to normalize Durations, or simply check that the input durations are valid.
| dur.Seconds *= -1 | ||
| dur.Nanos *= -1 | ||
| } | ||
|
|
There was a problem hiding this comment.
What about calling dur.CheckValid just to be sure our algo was correct?
There was a problem hiding this comment.
Outside the scope of Parse(). It doesn't need to handle anything outside of data formatted from a valid proto.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
gosec found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
@JimLarson Could you rebase/merge main, or give maintainers write access to your branch so that we can do it for you? |
Head branch was pushed to by a user without write access
JimLarson
force-pushed
the
jimlarson/12710-duration-renderer
branch
from
75f1184
to
837b72e
Compare
JimLarson
force-pushed
the
jimlarson/12710-duration-renderer
branch
from
837b72e
to
08731fe
Compare
|
@AmauryM or @julienrbrt please reenable the automerge. Thanks! |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #13312 +/- ##
==========================================
- Coverage 54.05% 53.97% -0.09%
==========================================
Files 653 648 -5
Lines 55899 55425 -474
==========================================
- Hits 30216 29915 -301
+ Misses 23269 23111 -158
+ Partials 2414 2399 -15
|
* feat: value renderer for durations * feat: dispatch for duration ValueRenderer * docs: update duration rendering spec * feat: error on malformed proto, review feedback, more tests * fix: silence spurious int overflow warnings * fix: supress spurious gosec warning on safe operation
Description
Closes: #12710
Adds SIGN_MODE_TEXTUAL value renderer for google.protobuf.Duration messages.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.md(CHANGELOG intentionally unmodified - will be updated with the whole epic)
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change