statesync: convert apphash to hex string in log#9591
statesync: convert apphash to hex string in log#9591mergify[bot] merged 10 commits intotendermint:mainfrom
Conversation
#9591 (comment) |
The rule of thumb is to use the |
972b497 to
7109548
Compare
|
there is only one place that might hit multiple times during statesync, changed to use |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
consensus/replay.go
Outdated
| h.logger.Info("ABCI Handshake App Info", | ||
| "height", blockHeight, | ||
| "hash", appHash, | ||
| "hash", fmt.Sprintf("%X", appHash), |
There was a problem hiding this comment.
In regards to:
The rule of thumb is to use the LazySprintf in positive paths (those that are hit often)
Isn't this an example of something that hits often i.e. once per start up (or would we not consider this often) (cc @sergio-mena )
There was a problem hiding this comment.
You're right. This is a "positive path".
@JayT106 would you mind using the LazySprintf here?
There was a problem hiding this comment.
In the interest of speeding this up, I've just done the change. Let me know if anything else is needed @sergio-mena else you can merge it
|
Thanks @thanethomson , I updated the branch to merge it several times, but then always got distracted with something else |
noticed this place is still printing unreadable apphash, convert to hex string
PR checklist
CHANGELOG_PENDING.mdupdated, or no changelog entry neededdocs/) and code comments, or nodocumentation updates needed