/bitcoin

Punctuation/grammer fixes in rpcwallet.cpp #10789

Merged
merged 1 commit into from Jul 25, 2017
+6 −6

Conversation

Reviewers

@jonasschnelli jonasschnelli

@TheBlueMatt TheBlueMatt

@morcos morcos

Assignees
No one assigned
Labels
Docs and Output
Projects
None yet
Milestone
No milestone
8 participants
@stevendlander @MeshCollider @morcos @jtimon @FilmCoder @TheBlueMatt @jonasschnelli @laanwj
@stevendlander
Contributor

stevendlander commented Jul 11, 2017

Standardizing punctuation on CLI output and also including a few fixes for grammer. This PR is for text only changes and includes no code edits.
@MeshCollider

MeshCollider commented Jul 11, 2017

NACK, disagree with the addition of full stops to every line and most of the changes in capitalization. Only a couple of the changes are valid spelling fixes, prefer if they were isolated from the rest

laanwj added the Refactoring label Jul 11, 2017

@jonasschnelli jonasschnelli added Docs and Output and removed Refactoring labels Jul 11, 2017

@stevendlander
Contributor

stevendlander commented Jul 11, 2017

Isolated spelling fixes and reverted all but one capitalization. Will revert that too if needed, thanks.
src/wallet/rpcwallet.cpp
@@ -719,7 +719,7 @@ UniValue getbalance(const JSONRPCRequest& request)
"\nExamples:\n"
"\nThe total amount in the wallet\n"
+ HelpExampleCli("getbalance", "") +
- "\nThe total amount in the wallet at least 5 blocks confirmed\n"
+ "\nThe total amount in the wallet at least 6 blocks confirmed\n"
@jonasschnelli

jonasschnelli Jul 11, 2017

Member

Ideally we include wallet.h's DEFAULT_TX_CONFIRM_TARGET here somehow.

@stevendlander

stevendlander Jul 11, 2017

Contributor

Should I code up a solution to that in this PR?

@morcos

morcos Jul 18, 2017

Contributor

No. It's a different concept

@stevendlander
Contributor

stevendlander commented Jul 11, 2017

I'll tidy up the places where the const char needs to be an unsigned int.
@stevendlander
Contributor

stevendlander commented Jul 11, 2017

Open to suggestions on a better approach for string-ifying DEFAULT_TX_CONFIRM_TARGET. I would imagine importing <string> just to do that is not ideal.

MeshCollider referenced this pull request Jul 13, 2017

Merged

getbalance example covers at least 6 confirms #10807

src/wallet/rpcwallet.cpp
@@ -587,10 +588,10 @@ UniValue getreceivedbyaddress(const JSONRPCRequest& request)
+ HelpExampleCli("getreceivedbyaddress", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\"") +
"\nThe amount including unconfirmed transactions, zero confirmations\n"
+ HelpExampleCli("getreceivedbyaddress", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\" 0") +
- "\nThe amount with at least 6 confirmation, very safe\n"
- + HelpExampleCli("getreceivedbyaddress", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\" 6") +
+ "\nThe amount with at least " + std::to_string(DEFAULT_TX_CONFIRM_TARGET) + " confirmation, very safe\n"
@TheBlueMatt

TheBlueMatt Jul 14, 2017

Contributor

lol, lets remove the "very safe" doge-speak...

@morcos
Contributor

morcos commented Jul 15, 2017

NACK.

I don't like tying the DEFAULT_TX_CONFIRM_TARGET to the concept of how many confirmations we might want to wait before considering a transaction to have low chance of being re-orged out. These are unrelated concepts that just both happen to use the number 6 right now.
@jtimon
Member

jtimon commented Jul 17, 2017 • edited

What about adding a different constant?
Needs rebase.
@stevendlander
Contributor

stevendlander commented Jul 19, 2017

Updated to address @morcos feedback.
@FilmCoder

FilmCoder commented Jul 19, 2017 • edited

Very grammar, much pull request, merge now wow.
@jtimon
Member

jtimon commented Jul 19, 2017

utACK e439beb
Please squash into one commit.
@stevendlander stevendlander Fix misspellings and remove safety verbiage
This commit was signed with a verified signature.
@stevendlander stevendlander Steven D. Lander
GPG key ID: 4DC1EDC5D4BE7A30 Learn about signing commits
a5ecaf1
@TheBlueMatt
Contributor

TheBlueMatt commented Jul 24, 2017

utACK a5ecaf1, we should take this for 15.

@laanwj laanwj merged commit a5ecaf1 into bitcoin:master Jul 25, 2017
1 check passed

1 check passed

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

@laanwj laanwj added a commit that referenced this pull request Jul 25, 2017

@laanwj laanwj Merge #10789: Punctuation/grammer fixes in rpcwallet.cpp 
a5ecaf1 Fix misspellings and remove safety verbiage (Steven D. Lander)

Pull request description:

  Standardizing punctuation on CLI output and also including a few fixes for grammer.  This PR is for text only changes and includes no code edits.

Tree-SHA512: afde551bf1212838822188b6723f2bf1b7222decfa1cd7aa6b04967489108a29f80833af6059252af028c53437755f258275af0614e0d4d0311e09421cd8e131
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 1E4AED62986CD25D Learn about signing commits
1124328
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment