Use constants and minor fixes by luke-jr#6905
Conversation
…database, not memory pool)
…t; also show correct default in getmininginfo
There was a problem hiding this comment.
Why this changes from -1 to GetArg("-genproclimit", DEFAULT_GENERATE_THREADS) but in getmininginfo() the change is from -1 to DEFAULT_GENERATE_THREADS?
There was a problem hiding this comment.
You still want to obey the command line arg if nothing is set via rpc.
There was a problem hiding this comment.
Is it a bugfix or enhanced functionality? Again, why one uses GetArg() but the other doesn't?
There was a problem hiding this comment.
Yes, it fixes one bug: If genproclimit is omitted to RPC setgenerate, don't change it; (copied from commit msg)
There was a problem hiding this comment.
Making getmininginfo push back the "correct" value nGenProcLimit may involve some more work. But at least this PR did not introduce the issue. The PR is not perfect, but improves things considerably.
There was a problem hiding this comment.
Thank you. Everything else was completely trivial to review, but I thought this deserved a more detailed explanation.
|
I think this is a good idea. Miner and Wallet init help strings could also slowly migrate into wallet/ miner/ classes. I once did this for the wallet, but it requires rebase and maybe more small, reviewable PRs: #5990 |
maflcko
force-pushed
the
lukejr-constants-no-mergeConf
branch
from
693de4d to
a6efc01
Compare
|
utACK |
|
utACK besides my nit/question. |
There was a problem hiding this comment.
Good find on missing ENABLE_WALLET here
|
utACK |
a6efc01 Bugfix: Omit wallet-related options from -help when wallet is disabled (Luke Dashjr) 5f9260f Bugfix: If genproclimit is omitted to RPC setgenerate, don't change it; also show correct default in getmininginfo (Luke Dashjr) 420a82f Bugfix: Describe dblogsize option correctly (it refers to the wallet database, not memory pool) (Luke Dashjr) caa3d42 Bugfix: RPC: blockchain: Display correct defaults in help for verifychain method (Luke Dashjr)
…hain method Github-Pull: bitcoin#6905 Rebased-From: caa3d42
…database, not memory pool) Github-Pull: bitcoin#6905 Rebased-From: 420a82f
…t; also show correct default in getmininginfo Github-Pull: bitcoin#6905 Rebased-From: 5f9260f
Github-Pull: bitcoin#6905 Rebased-From: a6efc01
…hain method Github-Pull: bitcoin#6905 Rebased-From: caa3d42
…database, not memory pool) Github-Pull: bitcoin#6905 Rebased-From: 420a82f
…t; also show correct default in getmininginfo Github-Pull: bitcoin#6905 Rebased-From: 5f9260f
Github-Pull: bitcoin#6905 Rebased-From: a6efc01
Misc upstream PRs Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6077 - Second commit only (first was already applied to 0.11.X and then reverted) - bitcoin/bitcoin#6284 - bitcoin/bitcoin#6489 - bitcoin/bitcoin#6462 - bitcoin/bitcoin#6647 - bitcoin/bitcoin#6235 - bitcoin/bitcoin#6905 - bitcoin/bitcoin#6780 - Excluding second commit (QT) and third commit (requires bitcoin/bitcoin#6993) - bitcoin/bitcoin#6961 - Excluding QT parts, and a small `src/policy/policy.cpp` change which depends on a bunch of other PRs, which we'll have to remember to come back to. - bitcoin/bitcoin#7044 - bitcoin/bitcoin#8856 - bitcoin/bitcoin#9002 Part of #2074 and #2132.
6 participants
Not sure if #6349 finds its way out of rebase hell anytime soon. Those three commits can be merged without rebase (I preserved the commit hashes)
They already got some sort of code review as part of #6349 so it's better to merge them without rebase, imo.