[Docs] Small updates to getrawtransaction description #15332

pull amitiuttarwar wants to merge 1 commits into bitcoin:master from amitiuttarwar:get_transaction_docs changing 2 files +15 −11

amitiuttarwar commented at 1:48 AM on February 3, 2019: contributor

As per review comments on https://github.com/bitcoin/bitcoin/pull/15159
fanquake added the label Docs on Feb 3, 2019
amitiuttarwar commented at 1:59 AM on February 3, 2019: contributor

Question about whitespace 😝- There seems to be an implicit convention of indenting the first line(s) of the RPCHelpMan descriptions and I'm wondering why? The changes look recent but I didn't find anything relevant in the style guide. If this is intentional, maybe I can add something there?

Ex: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpcdump.cpp#L113 & https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpcdump.cpp#L1196

However, this one doesn't extra-indent the first line: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpcdump.cpp#L755
MarcoFalke commented at 3:02 AM on February 3, 2019: member
There is no guideline about whitespace as long as you stick to either rule-of-thumb:
- install clang-format and run the https://github.com/bitcoin/bitcoin/tree/master/contrib/devtools#clang-format-diffpy script
- Keep the formatting as is

in src/rpc/rawtransaction.cpp:78 in 51d786879f outdated

  78 | +            "argument, getrawtransaction will return the transaction if the specified block is available and\n"
  79 | +            "the transaction is found in that block. When called without a blockhash argument, getrawtransaction\n"
  80 | +            "will return the transaction if it is in the mempool, or if -txindex is enabled and the transaction\n"
  81 | +            "is in a block in the blockchain.\n"
  82 | +
  83 | +            "\nThe gettransaction RPC can be used to retrieve wallet transactions.\n"

MarcoFalke commented at 3:06 AM on February 3, 2019:

Could add something like

                "\nHint: use getmempoolentry to fetch a specific transaction from the mempool.\n",

MarcoFalke commented at 3:07 AM on February 3, 2019: member

ACK
amitiuttarwar force-pushed on Feb 3, 2019
amitiuttarwar commented at 4:04 AM on February 3, 2019: contributor

hmm, ok went with format script if indentation was not intentional. added getmempoolentry reference.
kristapsk commented at 11:34 PM on February 3, 2019: contributor

utACK 35c7eb045f79b5a556c9d17eddf3bf59d0d1b55f
[Docs] Small updates to getrawtransaction description 47012391ec

in doc/release-notes.md:257 in 35c7eb045f outdated

 253 | @@ -254,11 +254,11 @@ in the Low-level Changes section below.
 254 |  
 255 |  - See the [Mining](#mining) section for changes to `getblocktemplate`.
 256 |  
 257 | -- The `getrawtransaction` RPC no longer checks the unspent UTXO set for
 258 | -  a transaction. The remaining behaviors are as follows: 1. If a
 259 | -  blockhash is provided, check the corresponding block. 2. If no
 260 | -  blockhash is provided, check the mempool. 3. If no blockhash is
 261 | -  provided but txindex is enabled, also check txindex.
 262 | +- The `getrawtransaction` RPC & REST endpoint no longer checks the

fanquake commented at 9:54 AM on February 4, 2019:

nit: RPC & REST endpoints no longer check

amitiuttarwar force-pushed on Feb 4, 2019
amitiuttarwar commented at 6:45 PM on February 5, 2019: contributor

This is ready.
MarcoFalke requested review from jnewbery on Feb 5, 2019

in doc/release-notes.md:258 in 47012391ec

 258 | -  a transaction. The remaining behaviors are as follows: 1. If a
 259 | -  blockhash is provided, check the corresponding block. 2. If no
 260 | -  blockhash is provided, check the mempool. 3. If no blockhash is
 261 | -  provided but txindex is enabled, also check txindex.
 262 | +- The `getrawtransaction` RPC & REST endpoints no longer check the
 263 | +  unspent UTXO set for a transaction. The remaining behaviors are as

jnewbery commented at 8:23 PM on February 5, 2019:

Nit: Perhaps this is fine in American English, but 'The remaining behaviors are...' sounds a little odd to me. 'The new behavior is...' sounds more natural.

amitiuttarwar commented at 9:24 PM on February 5, 2019:

I disagree, calling the behavior new seems misleading.

jnewbery commented at 10:16 PM on February 5, 2019:

I'll let others comment. ACK either way.

jnewbery commented at 8:24 PM on February 5, 2019: member

ACK 47012391ec25eb76669c7ba2aa458c3ac611289c.

One nit inline, which could be ignored.
MarcoFalke referenced this in commit baf125b31d on Feb 5, 2019
MarcoFalke merged this on Feb 5, 2019
MarcoFalke closed this on Feb 5, 2019
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#15159 [RPC] Remove lookup to UTXO set from GetTransaction