Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Blocktime format rounding #3894

Merged
merged 1 commit into from
Dec 20, 2016
Merged

Blocktime format rounding #3894

merged 1 commit into from
Dec 20, 2016

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Dec 19, 2016

Displays 13:57:47 as 15:37 as opposed to 15:38 (Reference now goes from blockTime -> current as opposed to the other way around)

May improve display for https://github.com/ethcore/parity/issues/3888

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Dec 19, 2016
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 6efaec8 on jg-fix-status-blocktime into ** on master**.

@derhuerst
Copy link
Contributor

Not sure, I understand this correctly. Cannot confirm that this fixes rounds 15:37:40 to 15:38.

@jacogr
Copy link
Contributor Author

jacogr commented Dec 19, 2016

Watch it with debugging info across 50 odd blocks pre-and-post logging the blocks numbers, blocktimes and moment display values with both versions - moment has differences between the rounding depending on what you go from and what you go to with .calendar(...). (current -> blocktime is different from blocktime -> current)

Pre this change it rounds up so you end up with displayed timestamps that has not happened yet.

@derhuerst
Copy link
Contributor

Pre this change it rounds up so you end up with displayed timestamps that has not happened yet.

Thanks for the clarification. Can confirm this PR fixes the issue.

@derhuerst derhuerst added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 20, 2016
@gavofyork gavofyork merged commit 8f5804e into master Dec 20, 2016
@gavofyork gavofyork deleted the jg-fix-status-blocktime branch December 20, 2016 16:35
@derhuerst derhuerst mentioned this pull request Dec 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants