Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tendermint: Added unix_timestamp and unix_timestamp_nanos method in Time struct. #1176

Merged
merged 2 commits into from
Nov 7, 2022
Merged

Tendermint: Added unix_timestamp and unix_timestamp_nanos method in Time struct. #1176

merged 2 commits into from
Nov 7, 2022

Conversation

scalalang2
Copy link
Contributor

@scalalang2 scalalang2 commented Aug 4, 2022

close #1175

I thought it would be easier to write code with other time libraries such as chrono if it porivided a unix timestamp

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Added entry in .changelog/

@scalalang2 scalalang2 changed the title Convert Time into unix timestamp Tendermint: Added to_timestamp and to_timestamp_nanos method in Time struct. Aug 4, 2022
@scalalang2 scalalang2 changed the title Tendermint: Added to_timestamp and to_timestamp_nanos method in Time struct. Tendermint: Added unix_timestamp and unix_timestamp_nanos method in Time struct. Aug 4, 2022
.changelog/v0.24.0-pre.1/improvements/1030-new-time-api.md Outdated Show resolved Hide resolved
tendermint/src/time.rs Outdated Show resolved Hide resolved
tendermint/src/time.rs Outdated Show resolved Hide resolved
@thanethomson thanethomson mentioned this pull request Aug 4, 2022
6 tasks
@scalalang2
Copy link
Contributor Author

I applied all feedbacks for now.

@scalalang2
Copy link
Contributor Author

scalalang2 commented Aug 11, 2022

Broken unit test is not related with this updates.
I'll fix it in another pull request.

mzabaluev
mzabaluev previously approved these changes Oct 29, 2022
Copy link
Contributor

@mzabaluev mzabaluev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link
Contributor

@thanethomson thanethomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase this against the main branch - we no longer use the master branch, as per #1179.

@scalalang2
Copy link
Contributor Author

Thanks for your feedback!
If the rebasing is done, i'll inform to you :)

@scalalang2 scalalang2 changed the base branch from master to main November 7, 2022 00:30
@scalalang2 scalalang2 changed the base branch from main to master November 7, 2022 00:32
@scalalang2 scalalang2 dismissed mzabaluev’s stale review November 7, 2022 00:32

The base branch was changed.

@scalalang2 scalalang2 changed the base branch from master to main November 7, 2022 00:33
@scalalang2
Copy link
Contributor Author

I rebased it, thanks for all reviews 👍

@codecov-commenter
Copy link

Codecov Report

Merging #1176 (5cd5f6b) into main (3c28657) will increase coverage by 0.0%.
The diff coverage is 84.2%.

@@          Coverage Diff          @@
##            main   #1176   +/-   ##
=====================================
  Coverage   64.2%   64.2%           
=====================================
  Files        244     244           
  Lines      21345   21364   +19     
=====================================
+ Hits       13715   13728   +13     
- Misses      7630    7636    +6     
Impacted Files Coverage Δ
tendermint/src/time.rs 97.6% <84.2%> (-1.0%) ⬇️
testgen/src/vote.rs 84.0% <0.0%> (-0.9%) ⬇️
testgen/src/commit.rs 90.6% <0.0%> (-0.7%) ⬇️
testgen/src/header.rs 82.9% <0.0%> (-0.6%) ⬇️
light-client-verifier/src/types.rs 38.4% <0.0%> (-0.3%) ⬇️
tendermint/src/node.rs 63.9% <0.0%> (+0.1%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@thanethomson thanethomson merged commit d102af4 into informalsystems:main Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: get timestamp for block
5 participants