Skip to content

#2877 include BlockTimeIota should be included in default config toml#2878

Merged
melekes merged 2 commits intotendermint:developfrom
cryptom-dev:BlockTimeIota_config_fix
Nov 19, 2018
Merged

#2877 include BlockTimeIota should be included in default config toml#2878
melekes merged 2 commits intotendermint:developfrom
cryptom-dev:BlockTimeIota_config_fix

Conversation

@cryptom-dev
Copy link
Contributor

@cryptom-dev cryptom-dev commented Nov 18, 2018

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

@codecov-io
Copy link

codecov-io commented Nov 18, 2018

Codecov Report

Merging #2878 into develop will increase coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #2878      +/-   ##
===========================================
+ Coverage     62.1%   62.15%   +0.04%     
===========================================
  Files          212      212              
  Lines        17288    17292       +4     
===========================================
+ Hits         10736    10747      +11     
+ Misses        5647     5639       -8     
- Partials       905      906       +1
Impacted Files Coverage Δ
config/toml.go 53.19% <ø> (ø) ⬆️
privval/ipc_server.go 64.15% <0%> (-5.67%) ⬇️
libs/events/events.go 93.2% <0%> (-4.86%) ⬇️
consensus/reactor.go 66.97% <0%> (+1.5%) ⬆️
evidence/reactor.go 62.62% <0%> (+1.57%) ⬆️
privval/tcp_server.go 81.42% <0%> (+2.85%) ⬆️

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

🌮

Thanks!

@melekes
Copy link
Contributor

melekes commented Nov 19, 2018

@milosevic is there a reason BlockTimeIota wasn't included in the config.toml file in the first place?

@cryptom-dev cryptom-dev force-pushed the BlockTimeIota_config_fix branch from de00283 to 38dd02b Compare November 19, 2018 14:52
@cryptom-dev cryptom-dev requested a review from zramsay as a code owner November 19, 2018 14:52
@milosevic
Copy link
Contributor

There is no reason why it hasn't been included; we most probably forgot to do it.

@melekes melekes merged commit fd8d1d6 into tendermint:develop Nov 19, 2018
peer_query_maj23_sleep_duration = "{{ .Consensus.PeerQueryMaj23SleepDuration }}"

# Block time parameters. Corresponds to the minimum time increment between consecutive blocks.
blocktime_iota = "{{ .Consensus.BlockTimeIota }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably change this to block_time_iota. Also the description doesn't help distinguish it from TimeoutCommit - should be clear that this refers to the timestamp.

Also, isn't it a ConsensusParam, rather than just a config option, @milosevic ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that description should clearly say this is only about timestamp. Yep it's a ConsensusParam (definition: ConsensusParams contains consensus critical parameters that determine the validity of blocks.).

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.

5 participants