Skip to content

Consistency fixes for RPC descriptions #14373

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 1 commit into from
Oct 10, 2018
Merged

Consistency fixes for RPC descriptions #14373

maflcko merged 1 commit into from
Oct 10, 2018

Conversation

@ch4ot1c
Copy link
Contributor

@ch4ot1c ch4ot1c commented Oct 2, 2018

No description provided.

practicalswift
practicalswift reviewed Oct 2, 2018
View reviewed changes
jimmysong
jimmysong reviewed Oct 3, 2018
View reviewed changes
Copy link
Contributor

@jimmysong jimmysong left a comment

Choose a reason for hiding this comment

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

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".

@maflcko
Copy link
Member

maflcko 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
Copy link
Contributor

DrahtBot 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
Copy link
Contributor Author

ch4ot1c commented Oct 7, 2018 •
edited
Loading

Also:

  • First letter capitalization consistency
  • Final . consistency

should be added to the docgen / its checker

@fanquake
Copy link
Member

fanquake 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 fix/rpc-descriptions branch from 942952d to a28dfe4 Compare October 9, 2018 13:40
@ch4ot1c
[rpc] Descriptions: Textual consistency fixes
b8edb98 
'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 fix/rpc-descriptions branch from a28dfe4 to b8edb98 Compare October 9, 2018 13:42
@ch4ot1c
Copy link
Contributor Author

ch4ot1c commented Oct 9, 2018

Squashed

@fanquake
Copy link
Member

fanquake commented Oct 10, 2018

utACK b8edb98

@meshcollider
Copy link
Contributor

meshcollider commented Oct 10, 2018

utACK b8edb98

@promag
Copy link
Contributor

promag 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
Copy link
Contributor

meshcollider 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
Copy link
Member

Sjors 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.

@maflcko maflcko merged commit b8edb98 into bitcoin:master Oct 10, 2018
maflcko pushed a commit that referenced this pull request Oct 10, 2018
Merge #14373: Consistency fixes for RPC descriptions
be99270 
b8edb98 [rpc] Descriptions: Textual consistency fixes (Jon Layton)

Pull request description:

Tree-SHA512: fa22ddac94e95672579cc84309f1c3ea3a2dbf762e45a8ae9c121c4c5188bf8c19ff9458d49dd7ef760c3ae4226487612a2954e9a1a0e8b720116afeb718b46b
@promag
Copy link
Contributor

promag commented Oct 10, 2018

Alrighty then 👍

@promag
Copy link
Contributor

promag commented Oct 10, 2018

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

@ch4ot1c ch4ot1c mentioned this pull request Oct 10, 2018
Closed
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 20, 2020
Merge #14373: Consistency fixes for RPC descriptions
9d64eab 
Summary:
b8edb9810a [rpc] Descriptions: Textual consistency fixes (Jon Layton)

Pull request description:

Tree-SHA512: fa22ddac94e95672579cc84309f1c3ea3a2dbf762e45a8ae9c121c4c5188bf8c19ff9458d49dd7ef760c3ae4226487612a2954e9a1a0e8b720116afeb718b46b

Backport of Core [[bitcoin/bitcoin#14373 | PR14373]]

Test Plan:
  ninja
  ninja check
  ./bitcoind
  ./bitcoin-cli help <changed rpcs>
Verify changes in help text.

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5517
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request May 19, 2020
@cculianu
Merge #14373: Consistency fixes for RPC descriptions
72a5275 
Summary:
b8edb9810a [rpc] Descriptions: Textual consistency fixes (Jon Layton)

Pull request description:

Tree-SHA512: fa22ddac94e95672579cc84309f1c3ea3a2dbf762e45a8ae9c121c4c5188bf8c19ff9458d49dd7ef760c3ae4226487612a2954e9a1a0e8b720116afeb718b46b

Backport of Core [[bitcoin/bitcoin#14373 | PR14373]]

Test Plan:
  ninja
  ninja check
  ./bitcoind
  ./bitcoin-cli help <changed rpcs>
Verify changes in help text.

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5517
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Jul 21, 2021
@Munkybooty
Merge bitcoin#14373: Consistency fixes for RPC descriptions
6c654d0 
b8edb98 [rpc] Descriptions: Textual consistency fixes (Jon Layton)

Pull request description:

Tree-SHA512: fa22ddac94e95672579cc84309f1c3ea3a2dbf762e45a8ae9c121c4c5188bf8c19ff9458d49dd7ef760c3ae4226487612a2954e9a1a0e8b720116afeb718b46b
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Jul 23, 2021
@Munkybooty
Merge bitcoin#14373: Consistency fixes for RPC descriptions
2284dd0 
b8edb98 [rpc] Descriptions: Textual consistency fixes (Jon Layton)

Pull request description:

Tree-SHA512: fa22ddac94e95672579cc84309f1c3ea3a2dbf762e45a8ae9c121c4c5188bf8c19ff9458d49dd7ef760c3ae4226487612a2954e9a1a0e8b720116afeb718b46b
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@jimmysong jimmysong jimmysong left review comments

@practicalswift practicalswift practicalswift left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

RPC/REST/ZMQ

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@ch4ot1c @maflcko @DrahtBot @fanquake @meshcollider @promag @Sjors @jimmysong @practicalswift