cosmos-sdk icon indicating copy to clipboard operation
cosmos-sdk copied to clipboard

feat: value renderer for timestamp protos

Open JimLarson opened this issue 3 years ago • 1 comments

Description

Closes: #12709

Part of Sign Mode Textual (ADR 050) implementation.

Renders Timestamp messages as RFC 3339 (simplified ISO 8601).


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...

  • [ ] included the correct type prefix in the PR title
  • [ ] added ! to the type prefix if API or client breaking change
  • [ ] targeted the correct branch (see PR Targeting)
  • [ ] provided a link to the relevant issue or specification
  • [ ] followed the guidelines for building modules
  • [ ] included the necessary unit and integration tests
  • [ ] added a changelog entry to CHANGELOG.md
  • [ ] included comments for documenting Go code
  • [ ] updated the relevant documentation or specification
  • [ ] reviewed "Files changed" and left comments if necessary
  • [ ] confirmed all CI checks have passed

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...

  • [ ] confirmed the correct type prefix in the PR title
  • [ ] confirmed ! in the type prefix if API or client breaking change
  • [ ] confirmed all author checklist items have been addressed
  • [ ] reviewed state machine logic
  • [ ] reviewed API design and naming
  • [ ] reviewed documentation is accurate
  • [ ] reviewed tests and test coverage
  • [ ] manually tested (if applicable)

JimLarson avatar Aug 08 '22 19:08 JimLarson

Codecov Report

Merging #12860 (48fd2d0) into main (4fe7797) will decrease coverage by 0.13%. The diff coverage is 63.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #12860      +/-   ##
==========================================
- Coverage   55.87%   55.74%   -0.14%     
==========================================
  Files         646      649       +3     
  Lines       54895    55022     +127     
==========================================
- Hits        30675    30671       -4     
- Misses      21762    21880     +118     
- Partials     2458     2471      +13     
Impacted Files Coverage Δ
baseapp/abci.go 64.09% <0.00%> (-0.33%) :arrow_down:
client/flags/flags.go 19.35% <0.00%> (-0.32%) :arrow_down:
client/rpc/status.go 66.66% <ø> (ø)
client/utils.go 34.92% <0.00%> (ø)
server/config/config.go 38.00% <0.00%> (-1.59%) :arrow_down:
server/rosetta/client_online.go 1.35% <0.00%> (ø)
server/swagger.go 0.00% <0.00%> (ø)
testutil/list.go 0.00% <0.00%> (ø)
types/result.go 76.74% <ø> (ø)
x/auth/tx/query.go 0.00% <ø> (ø)
... and 69 more

codecov[bot] avatar Aug 08 '22 19:08 codecov[bot]

Reviewers PTAL - addressed all comments.

JimLarson avatar Aug 18 '22 04:08 JimLarson

Added the rationale for fractional second truncation vs sortability.

Also confirmed 100% test coverage for the added code (except for the TODO for handling default proto messages).

JimLarson avatar Aug 19 '22 17:08 JimLarson

Core reviewer - ready for your approval.

Changelog update intentionally omitted - we'll have one entry for the whole functionality.

JimLarson avatar Aug 22 '22 23:08 JimLarson

I put automerge on.

This branch is out-of-date with the base branch

@JimLarson could you rebase/merge master into your branch? Or alternatively give write access to your branch, so that our bot can do it.

amaury1093 avatar Aug 23 '22 14:08 amaury1093