luke-jr constants #6961

MarcoFalke commented at 11:28 AM on November 6, 2015: member

Rebased version of #6349 (2/3).

I have dropped the changes to src/chainparamsbase.cpp to only include code that has already been reviewed as part of #6349.

in src/init.cpp:None in 82fcf8b501 outdated

 349 | +    strUsage += HelpMessageOpt("-bantime=<n>", strprintf(_("Number of seconds to keep misbehaving peers from reconnecting (default: %u)"), DEFAULT_MISBEHAVING_BANTIME));
 350 |      strUsage += HelpMessageOpt("-bind=<addr>", _("Bind to given address and always listen on it. Use [host]:port notation for IPv6"));
 351 |      strUsage += HelpMessageOpt("-connect=<ip>", _("Connect only to the specified node(s)"));
 352 |      strUsage += HelpMessageOpt("-discover", _("Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)"));
 353 | -    strUsage += HelpMessageOpt("-dns", _("Allow DNS lookups for -addnode, -seednode and -connect") + " " + _("(default: 1)"));
 354 | +    strUsage += HelpMessageOpt("-dns", strprintf(_("Allow DNS lookups for -addnode, -seednode and -connect (default: %u)"), fNameLookup));

MarcoFalke commented at 12:00 PM on November 6, 2015:

Nit: Reuse translation?

in src/init.cpp:None in 82fcf8b501 outdated

  64 | @@ -64,6 +65,10 @@ using namespace std;
  65 |  CWallet* pwalletMain = NULL;
  66 |  #endif
  67 |  bool fFeeEstimatesInitialized = false;
  68 | +static const bool DEFAULT_PROXYRANDOMIZE = true;
  69 | +static const bool DEFAULT_REST_ENABLE = false;
  70 | +static const bool DEFAULT_SAFEMODE = true;

laanwj commented at 1:14 PM on November 6, 2015:

I'd prefer to use DEFAULT_DISABLESAFEMODE = false then use that instead of negating this everywhere it is used. (agree that in retrospect the option should have been called safemode and acted in this way, but there's no way back now, and having this consistent makes it easier to link it to the constant)

dcousens commented at 1:14 PM on November 6, 2015: contributor

utACK

in src/util.h:None in 82fcf8b501 outdated

  50 | @@ -51,6 +51,13 @@ extern bool fLogIPs;
  51 |  extern volatile bool fReopenDebugLog;
  52 |  extern CTranslationInterface translationInterface;
  53 |  
  54 | +extern const char * const BITCOIN_CONF_FILENAME;
  55 | +extern const char * const BITCOIN_PID_FILENAME;
  56 | +
  57 | +static const bool DEFAULT_SELFSIGNED_ROOTCERTS = false;
  58 | +static const bool DEFAULT_CHOOSE_DATADIR = false;

laanwj commented at 1:16 PM on November 6, 2015:

It's ugly to have defaults for these GUI specific options in the core, they are not related to util at all. IMO the help for GUI specific options should be moved to the GUI part of the code.

dcousens commented at 2:52 AM on November 7, 2015:

Agreed, plus if disable wallet is enabled, it'll possibly throw a tonne of unused variable warnings (assuming -Wnounused)

dcousens commented at 2:53 AM on November 7, 2015:

For reference, @laanwj you meant DEFAULT_SPLASHSCREEN (and DEFAULT_CHOOSE_DATADIR?)?

MarcoFalke commented at 4:57 PM on November 7, 2015:

@laanwj @dcousens So where would I put those? qt/guiconstants.h? Or ratherqt/paymentrequestplus.h, qt/intro.h ...

laanwj added the label Refactoring on Nov 6, 2015

MarcoFalke force-pushed on Nov 6, 2015

MarcoFalke force-pushed on Nov 7, 2015

MarcoFalke force-pushed on Nov 9, 2015

MarcoFalke commented at 11:44 AM on November 9, 2015: member

Rebased. (Due to the translations cleanup) @laanwj Is it ok with the gui constants like this?

laanwj commented at 2:27 PM on November 9, 2015: member

I don't like including a GUI header from the core. We don't do that anywhere.

I'd prefer moving the part of HelpMessage with GUI specific settings to the GUI code.

MarcoFalke force-pushed on Nov 9, 2015

MarcoFalke commented at 7:38 PM on November 9, 2015: member

I put them in HelpMessageDialog()

laanwj commented at 2:52 PM on November 10, 2015: member

I put them in HelpMessageDialog()

That's awesome. They originally came from there and they shouldn't have been moved. Thanks!

~~ACK~~

in src/netbase.cpp:None in 2582225ca1 outdated

  39 | @@ -40,7 +40,7 @@ static proxyType proxyInfo[NET_MAX];
  40 |  static proxyType nameProxy;
  41 |  static CCriticalSection cs_proxyInfos;
  42 |  int nConnectTimeout = DEFAULT_CONNECT_TIMEOUT;
  43 | -bool fNameLookup = false;

laanwj commented at 2:54 PM on November 10, 2015:

~~Why do you change this default?~~ ok it isn't changed it was always true

in src/init.cpp:None in 2582225ca1 outdated

1125 | @@ -1134,7 +1126,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
1126 |      // see Step 2: parameter interactions for more information about these
1127 |      fListen = GetBoolArg("-listen", DEFAULT_LISTEN);
1128 |      fDiscover = GetBoolArg("-discover", true);
1129 | -    fNameLookup = GetBoolArg("-dns", true);
1130 | +    fNameLookup = GetBoolArg("-dns", fNameLookup);

laanwj commented at 2:54 PM on November 10, 2015:

Please define DEFAULT_* constant for this, instead of passing through the old value of the global.

MarcoFalke commented at 10:24 PM on November 10, 2015:

@laanwj Done.

MarcoFalke commented at 11:00 AM on November 12, 2015: member

Anything holding this back?

MarcoFalke force-pushed on Nov 12, 2015

MarcoFalke force-pushed on Nov 16, 2015

MarcoFalke force-pushed on Nov 17, 2015

MarcoFalke force-pushed on Nov 26, 2015

MarcoFalke commented at 9:50 PM on November 26, 2015: member

Rebased

sipa commented at 12:51 AM on November 28, 2015: member

Concept ACK, I haven't checked the code changes in detail yet, but needs rebase (again...).

Constrain constant values to a single location in code b966aa836a

Bugfix: Omit wallet-related options from -help when wallet is not supported 3307bdb333

[trivial] Reuse translation and cleanup DEFAULT_* values

* DEFAULT_DISABLE_SAFEMODE = false
* Use DEFAULT_* constants for extern bools

faf93f37fe

[qt] Move GUI related HelpMessage() part downstream fa41d4c8c6

MarcoFalke force-pushed on Nov 28, 2015

MarcoFalke commented at 8:47 PM on November 28, 2015: member

Rebased.

I haven't checked the code changes in detail yet, but needs rebase (again...).

This needs rebase approximately every 4 days. I checked the code as of #6349 and verified each rebase twice. Though, I don't think constant rebase makes this PR any better.

in src/util.h:None in fa41d4c8c6

  28 | @@ -29,6 +29,8 @@
  29 |  #include <boost/thread/exceptions.hpp>
  30 |  
  31 |  static const bool DEFAULT_LOGTIMEMICROS = false;
  32 | +static const bool DEFAULT_LOGIPS        = false;
  33 | +static const bool DEFAULT_LOGTIMESTAMPS = true;

sipa commented at 9:04 PM on November 28, 2015:

This was false.

sipa commented at 9:07 PM on November 28, 2015:

Never mind. The fLogTimestamps variable was initialized to false before, but the default setting was true. This makes it more consistent. Good.

sipa commented at 9:12 PM on November 28, 2015: member

ACK. Read over all the code changes, and compared the output of bitcoind and bitcoin-qt's --help output.

sipa merged this on Nov 28, 2015

sipa closed this on Nov 28, 2015

sipa referenced this in commit 8d26289c9a on Nov 28, 2015

MarcoFalke deleted the branch on Nov 28, 2015

luke-jr referenced this in commit a1c27c33f0 on Nov 30, 2015

luke-jr referenced this in commit 72c9ac6180 on Dec 8, 2015

zkbot referenced this in commit f1aeaec471 on Mar 21, 2018

str4d referenced this in commit 5c76b1df64 on Nov 14, 2019

str4d referenced this in commit de2eddd21d on Dec 3, 2019

zkbot referenced this in commit 4fc490c430 on Dec 4, 2019

zkbot referenced this in commit 868c63f92d on Dec 4, 2019

random-zebra referenced this in commit e89e20eca1 on Aug 25, 2020

MarcoFalke locked this on Sep 8, 2021