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

Consistency fixes for RPC descriptions #14373

Merged
merged 1 commit into from Oct 10, 2018

Conversation

Projects
None yet
9 participants
@ch4ot1c
Copy link
Contributor

commented Oct 2, 2018

No description provided.

@jimmysong
Copy link
Contributor

left a comment

It would be nice to get all the capitalizations consistent as well. Sometimes the description for a hash result will say "The block hash" others will say "the block hash".

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Oct 3, 2018

The formatting fixes would be more elegantly solved with a helper class that takes care of the formatting: See #14378

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2018

Coverage Change (pull 14373) Reference (master)
Lines -0.0463 % 87.0471 %
Functions +0.0000 % 84.1130 %
Branches -0.0322 % 51.5403 %
@ch4ot1c

This comment has been minimized.

Copy link
Contributor Author

commented Oct 7, 2018

Also:

  • First letter capitalization consistency
  • Final . consistency

should be added to the docgen / its checker

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 9, 2018

Please squash your commits, and we can merge this in. Moving forward we're likely to be generating this kind of documentation, so that styling can remain consistent.

@ch4ot1c ch4ot1c force-pushed the ch4ot1c:fix/rpc-descriptions branch Oct 9, 2018

[rpc] Descriptions: Textual consistency fixes
'Must be one of' should always end in a ':'

'hex encoded' is now always 'hex-encoded'

Remove redundant '(defaults to CONSERVATIVE)' text from estimatesmartfee

Consistent spacing for options '( verbose )' and '( verbosity )'

'BIP125 replaceable' is now always 'BIP125-replaceable'

JSON-RPC example is now always 'As a JSON-RPC call'

@ch4ot1c ch4ot1c force-pushed the ch4ot1c:fix/rpc-descriptions branch to b8edb98 Oct 9, 2018

@ch4ot1c

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2018

Squashed

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

utACK b8edb98

@meshcollider

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

utACK b8edb98

@promag

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

It would be nice to have a plan for this consistency/cleanup/format etc. There is nothing in place to prevent adding the same inconsistencies. As it is I'm -0.

@meshcollider

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

@promag: @achow101 is working on auto-generated help texts for the RPCs so there is a plan in place for the future, this should be the last RPC help text change before that is done

@Sjors

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

utACK b8edb98

@promag this seems like a strict improvement even without a plan. However I wouldn't want to review these kinds of pull requests too often, so looking forward to @achow101's work.

@fanquake fanquake added this to Mergeable in High-priority for review Oct 10, 2018

@MarcoFalke MarcoFalke merged commit b8edb98 into bitcoin:master Oct 10, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Oct 10, 2018

Merge #14373: Consistency fixes for RPC descriptions
b8edb98 [rpc] Descriptions: Textual consistency fixes (Jon Layton)

Pull request description:

Tree-SHA512: fa22ddac94e95672579cc84309f1c3ea3a2dbf762e45a8ae9c121c4c5188bf8c19ff9458d49dd7ef760c3ae4226487612a2954e9a1a0e8b720116afeb718b46b
@promag

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

Alrighty then 👍

@promag

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

Missed "listtransactions (dummy count skip include_watchonly)\n"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.