scripted-diff: prefix [address|change]type parameters with 'default' #12216

1. 
   Sjors commented at 1:37 PM on January 18, 2018: member

   Making it clear that these parameters can be overridden by individual wallet commands.

   Requesting 0.16 tag, as it's probably not worth changing otherwise.

   This makes grateful use of the fact that @sipa used an underscore for address_type and change_type RPC arguments.

2. MarcoFalke added this to the milestone 0.16.0 on Jan 18, 2018
3. MarcoFalke added the label Docs on Jan 18, 2018
4. scripted-diff: prefix [address|change]type parameters with 'default'

   Making it clear that these parameters can be overridden by individual wallet commands.
   
   -BEGIN VERIFY SCRIPT-
   sed -i 's/addresstype/defaultaddresstype/g' doc/*.md src/wallet/*.cpp test/functional/*.py
   sed -i 's/changetype/defaultchangetype/g' doc/*.md src/wallet/*.cpp test/functional/*.py
   -END VERIFY SCRIPT-

   e47af5452e
5. Sjors force-pushed on Jan 18, 2018
6. 
   Sjors commented at 1:47 PM on January 18, 2018: member

   @MarcoFalke this changes more than just docs, probably needs a wallet and RPC label.

7. 
   ryanofsky commented at 4:17 PM on January 18, 2018: member

   utACK e47af5452ef15f0afff633110ba857fa5006e516. I think this is nice to make it clear that these options won't somehow cause the bitcoin node to dictate an address type.

8. in src/wallet/init.cpp:19 in e47af5452e

     15 | @@ -16,8 +16,8 @@
     16 |  std::string GetWalletHelpString(bool showDebug)
     17 |  {
     18 |      std::string strUsage = HelpMessageGroup(_("Wallet options:"));
     19 | -    strUsage += HelpMessageOpt("-addresstype", strprintf(_("What type of addresses to use (\"legacy\", \"p2sh-segwit\", or \"bech32\", default: \"%s\")"), FormatOutputType(OUTPUT_TYPE_DEFAULT)));
   

   ---

   ---

   

   promag commented at 5:55 PM on January 18, 2018:

   I would rather improve the description.

   BTW, currently RPC help is very clear:

   https://github.com/bitcoin/bitcoin/blob/898f560b55aba2070f188b0223ef2beefcdede8b/src/wallet/rpcwallet.cpp#L147

9. ryanofsky referenced this in commit 23fc59a941 on Jan 18, 2018
10. laanwj removed this from the milestone 0.16.0 on Jan 18, 2018
11. 
    laanwj commented at 7:31 PM on January 18, 2018: member

    Removing this from 0.16 milestone - during the IRC meeting it was found that many options apply to defaults, so adding default here would be inconsistent. Would be better to improve the help message instead.

12. 
    jonasschnelli commented at 7:33 PM on January 18, 2018: contributor

    Should be closed then because changing the arguments after a release is a no-go.

13. MarcoFalke closed this on Jan 18, 2018

    ---

14. MarcoFalke added this to the milestone 0.16.0 on Jan 18, 2018
15. Sjors deleted the branch on Jan 19, 2018
16. 
    Sjors commented at 8:01 AM on January 19, 2018: member

    Agree with the above, as well as @laanwj's point "shorter option names are easier to remember/type". The behavior of these parameters did lead to some confusion during code review, but with the proper docs hopefully it will be fine for users.

17. DrahtBot locked this on Sep 8, 2021

Contributors

Sjorsryanofskypromaglaanwjjonasschnelli

Milestone
0.16.0