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

rpc: Correctly name arguments #14720

Merged
merged 1 commit into from Nov 13, 2018

Conversation

Projects
None yet
7 participants
@MarcoFalke
Copy link
Member

commented Nov 13, 2018

Consistently use the same name to describe arguments in the documentation and add a test that uses the name.

By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to RPCHelpMan.

The tests should pass with or without the changes in src.

Partly stolen from #14459 (More RPC help description fixes by ch4ot1c)

@MarcoFalke

This comment has been minimized.

Copy link
Member Author

commented Nov 13, 2018

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:Mf1811-rpcNameArguments branch Nov 13, 2018

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:Mf1811-rpcNameArguments branch to fa0815c Nov 13, 2018

@meshcollider

This comment has been minimized.

Copy link
Member

commented Nov 13, 2018

utACK fa0815c

@practicalswift

This comment has been minimized.

Copy link
Member

commented Nov 13, 2018

Concept ACK

Nice cleanup!

@laanwj

This comment has been minimized.

Copy link
Member

commented Nov 13, 2018

Makes sense, though I guess the idea eventually would be to use the just-merged RPCHelpMan here?

The tests should pass with or without the changes in src.

Might make sense to split them out to a commit that goes in before the src changes to make this clear.

@MarcoFalke

This comment has been minimized.

Copy link
Member Author

commented Nov 13, 2018

By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to RPCHelpMan.

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2018

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #14411 ([wallet] Restore ability to list incoming transactions by label by ryanofsky)
  • #13756 (wallet: "avoid_reuse" wallet flag for improved privacy by kallewoof)
  • #12674 (RPC: Support addnode onetry without making the connection priviliged by luke-jr)
  • #10593 (Relax punishment for peers relaying invalid blocks and headers by luke-jr)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@ryanofsky
Copy link
Contributor

left a comment

utACK fa0815c

@MarcoFalke MarcoFalke merged commit fa0815c into bitcoin:master Nov 13, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Nov 13, 2018

Merge #14720: rpc: Correctly name arguments
fa0815c rpc: Correctly name arguments (Jon Layton)

Pull request description:

  Consistently use the same name to describe arguments in the documentation and add a test that uses the name.

  By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to `RPCHelpMan`.

  The tests should pass with or without the changes in `src`.

  Partly stolen from #14459 (More RPC help description fixes by ch4ot1c)

Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c

@MarcoFalke MarcoFalke deleted the MarcoFalke:Mf1811-rpcNameArguments branch Nov 13, 2018

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.