luke-jr constants by maflcko · Pull Request #6961 · bitcoin/bitcoin

maflcko · 2015-11-06T11:28:32Z

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.

maflcko · 2015-11-06T12:00:44Z

src/init.cpp

Nit: Reuse translation?

dcousens · 2015-11-06T13:14:59Z

utACK

laanwj · 2015-11-06T13:16:39Z

src/util.h

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.

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

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

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

maflcko · 2015-11-09T11:44:23Z

Rebased. (Due to the translations cleanup)

@laanwj Is it ok with the gui constants like this?

laanwj · 2015-11-09T14:27:20Z

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.

maflcko · 2015-11-09T19:38:03Z

I put them in HelpMessageDialog()

laanwj · 2015-11-10T14:52:35Z

I put them in HelpMessageDialog()

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

~~ACK~~

laanwj · 2015-11-10T14:54:13Z

src/netbase.cpp

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

maflcko · 2015-11-12T11:00:19Z

Anything holding this back?

maflcko · 2015-11-26T21:50:16Z

Rebased

sipa · 2015-11-28T00:51:42Z

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

…ported

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

maflcko · 2015-11-28T20:47:51Z

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.

sipa · 2015-11-28T21:04:56Z

src/util.h

This was false.

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

sipa · 2015-11-28T21:12:23Z

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

fa41d4c [qt] Move GUI related HelpMessage() part downstream (MarcoFalke) faf93f3 [trivial] Reuse translation and cleanup DEFAULT_* values (MarcoFalke) 3307bdb Bugfix: Omit wallet-related options from -help when wallet is not supported (Luke Dashjr) b966aa8 Constrain constant values to a single location in code (Luke Dashjr)

…ported Github-Pull: bitcoin#6961 Rebased-From: 3307bdb

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.

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#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.

7e71759 Wallet: Split main logic from InitLoadWallet into CreateWalletFromFile (random-zebra) e585dad [Wallet] refactor wallet/init interaction (random-zebra) 49646b2 [Refactor] Nuke zwalletMain global object (random-zebra) f5f9df9 Wallet: Let the interval-flushing thread figure out the filename (Luke Dashjr) 74445e4 [wallet] Move hardcoded file name out of log messages (random-zebra) 3f1838d [Wallet] optimize return value of InitLoadWallet() (random-zebra) 539dea4 [Wallet] move "load wallet phase" to CWallet (random-zebra) 7644318 [Wallet] move wallet help string creation to CWallet (random-zebra) a9d69b8 [trivial] Reuse translation and cleanup DEFAULT_* values (random-zebra) cfd007a [Bug] Omit wallet-related options from -help when wallet not supported (random-zebra) da642e6 [Refactor] More constant default values cleanup (random-zebra) a45275a [Refactor] Implement CBaseChainParams::BaseParams(Network) (random-zebra) 09abb98 Constrain constant values to a single location in code (random-zebra) Pull request description: Adapts the following refactoring PRs: - bitcoin#6961 Constrain constant values to a single location in code - bitcoin#7576 [Wallet] move wallet help string creation to CWallet - bitcoin#7577 move "load wallet phase" to CWallet - bitcoin#7608 Move hardcoded file name out of log messages - bitcoin#7691 refactor wallet/init interaction - bitcoin#8776 Wallet refactoring leading up to multiwallet ACKs for top commit: furszy: re ACK 7e71759 Fuzzbawls: ACK 7e71759 Tree-SHA512: 5fad328b9ddf8187af97d3d5fb285d0b67e73d51ac1bc44a3d57d0af86bce8b34efaab539b7bdbc4f9a2fa575a936f83788cffcc9c6d6d04cd3e63b19d399400

maflcko reviewed Nov 6, 2015
View reviewed changes

src/init.cpp Outdated

Copy link

Member Author

maflcko Nov 6, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Reuse translation?

laanwj reviewed Nov 6, 2015
View reviewed changes

laanwj added the Refactoring label Nov 6, 2015

maflcko force-pushed the luke-jr-const branch 4 times, most recently from 16aeda9 to b10044d Compare November 9, 2015 11:42

maflcko force-pushed the luke-jr-const branch 3 times, most recently from 3ab6eb5 to 2582225 Compare November 9, 2015 18:55

laanwj reviewed Nov 10, 2015
View reviewed changes

src/netbase.cpp

Copy link

Member

laanwj Nov 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

maflcko force-pushed the luke-jr-const branch 3 times, most recently from 1beb3fc to cfaf3b8 Compare November 17, 2015 11:54

maflcko force-pushed the luke-jr-const branch from cfaf3b8 to faa0ef3 Compare November 26, 2015 20:55

luke-jr and others added 4 commits November 28, 2015 18:47

Constrain constant values to a single location in code

b966aa8

Bugfix: Omit wallet-related options from -help when wallet is not sup…

3307bdb

…ported

[trivial] Reuse translation and cleanup DEFAULT_* values

faf93f3

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

[qt] Move GUI related HelpMessage() part downstream

fa41d4c

maflcko force-pushed the luke-jr-const branch from faa0ef3 to fa41d4c Compare November 28, 2015 18:02

sipa reviewed Nov 28, 2015
View reviewed changes

sipa merged commit fa41d4c into bitcoin:master Nov 28, 2015

maflcko deleted the luke-jr-const branch November 28, 2015 21:15

sipa mentioned this pull request Nov 28, 2015

Constrain constant values to a single location in code #6349

Closed

luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Nov 30, 2015

Bugfix: Omit wallet-related options from -help when wallet is not sup…

a1c27c3

…ported Github-Pull: bitcoin#6961 Rebased-From: 3307bdb

jtimon mentioned this pull request Dec 3, 2015

Chainparams: Use a regular factory for creating chainparams #6907

Closed

luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Dec 8, 2015

Bugfix: Omit wallet-related options from -help when wallet is not sup…

72c9ac6

…ported Github-Pull: bitcoin#6961 Rebased-From: 3307bdb

This was referenced May 17, 2017

mapArgs is modified by setgenerate RPC without locking zcash/zcash#2132

Open

Misc upstream PRs zcash/zcash#2390

Merged

random-zebra mentioned this pull request Aug 17, 2020

[Wallet][Refactoring] wallet/init refactoring backports PIVX-Project/PIVX#1811

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Conversation

maflcko commented Nov 6, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcousens commented Nov 6, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented Nov 9, 2015

Uh oh!

laanwj commented Nov 9, 2015

Uh oh!

maflcko commented Nov 9, 2015

Uh oh!

laanwj commented Nov 10, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented Nov 12, 2015

Uh oh!

maflcko commented Nov 26, 2015

Uh oh!

sipa commented Nov 28, 2015

Uh oh!

maflcko commented Nov 28, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sipa commented Nov 28, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants