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

rpcserver: Add median time to verbose results. #2638

Merged
merged 1 commit into from Apr 22, 2021

Conversation

JoeGruffins
Copy link
Member

Median time is a useful value to return to clients, saving them the
trouble of calculating it themselves. Include the value in verbose
getblock and getblockheader results.

closes #2637

@JoeGruffins JoeGruffins marked this pull request as ready for review April 21, 2021 04:57
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. This looks pretty good overall. Just a couple of inline comments.

blockchain/chain.go Outdated Show resolved Hide resolved
internal/rpcserver/rpcserver.go Outdated Show resolved Hide resolved
@davecgh davecgh added this to the 1.7.0 milestone Apr 21, 2021
Copy link
Member

@rstaudt2 rstaudt2 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, I tested getblock and getblockheader, and they worked as expected. The JSON RPC API docs should probably also be updated to include the newly added field.

@davecgh
Copy link
Member

davecgh commented Apr 21, 2021

The JSON RPC API docs should probably also be updated to include the newly added field.

Good call. Agreed.

@JoeGruffins
Copy link
Member Author

Will update the docs.

Median time is a useful value to return to clients, saving them the
trouble of calculating it themselves. Include the value in verbose
getblock and getblockheader results.
@JoeGruffins
Copy link
Member Author

Added to docs/json_rpc_api.mediawiki in two places. I think that's sufficient?

@JoeGruffins
Copy link
Member Author

With the internal help, all phrases start with a lowercase "the" and end with a period. In the docs, they start with a capitalized "The" and have no ending punctuation. Just wanted to say that out loud.

@davecgh
Copy link
Member

davecgh commented Apr 22, 2021

Added to docs/json_rpc_api.mediawiki in two places. I think that's sufficient?

Yes, the changes look good. One for each of the two modified results.

With the internal help, all phrases start with a lowercase "the" and end with a period. In the docs, they start with a capitalized "The" and have no ending punctuation. Just wanted to say that out loud.

This is correct. They both follow the standard practices for their respective function. Namely, the help output follows standard JSON-RPC practices while the API documentation follows standard API docs practices of using full grammatically-correct sentences.

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Approval with JSON-RPC API documentation updates.

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.

rpcserver: Add mediantime to getblock and getblockheader verbose results.
3 participants