Skip to content

Make "Update to validators" msg value pretty #2848

Merged
ebuchman merged 6 commits intotendermint:developfrom
danil-lashin:2815-validators-update-log-format
Nov 16, 2018
Merged

Make "Update to validators" msg value pretty #2848
ebuchman merged 6 commits intotendermint:developfrom
danil-lashin:2815-validators-update-log-format

Conversation

@danil-lashin
Copy link
Contributor

Fixes #2765

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


// updateState returns a new State updated according to the header and responses.
func updateState(
logger log.Logger,
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh - we were probably logging it where we were before to avoid having to pass the logger here.

Not sure what exactly would be better here - but we do want to do a better job of separating our logging from our logic.

Maybe the abci responses should be processed into a new form (ie. call all the PB2TM) before calling this? Then the logging can happen before calling this as well and we wouldn't need to pass the logger in.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. Think that such kind of refactoring deserves separate issue/PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Opened #2865 - thanks!

@codecov-io
Copy link

codecov-io commented Nov 16, 2018

Codecov Report

Merging #2848 into develop will increase coverage by 0.01%.
The diff coverage is 80%.

@@             Coverage Diff             @@
##           develop    #2848      +/-   ##
===========================================
+ Coverage    62.16%   62.18%   +0.01%     
===========================================
  Files          212      212              
  Lines        17227    17230       +3     
===========================================
+ Hits         10710    10714       +4     
  Misses        5614     5614              
+ Partials       903      902       -1
Impacted Files Coverage Δ
state/execution.go 75.45% <80%> (+0.33%) ⬆️
privval/ipc_server.go 64.15% <0%> (-5.67%) ⬇️
privval/remote_signer.go 82.85% <0%> (ø) ⬆️
consensus/reactor.go 66.39% <0%> (+0.11%) ⬆️
privval/tcp_server.go 85.71% <0%> (+4.28%) ⬆️

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.

🌮

@ebuchman ebuchman merged commit 2cfdef6 into tendermint:develop Nov 16, 2018
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