Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

init: Remove deprecated args from hidden args #14272

Merged
merged 1 commit into from Sep 20, 2018

Conversation

Projects
None yet
9 participants
@MarcoFalke
Copy link
Member

commented Sep 19, 2018

The args have been deprecated since 0.17 (maybe longer) and since we reject unknown args, there is no need to add deprecated args to the list of hidden args and then hand-craft an error message if a user provides them.

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:Mf1809-initDeprecatedArgs branch to fa910e4 Sep 19, 2018

@ken2812221

This comment has been minimized.

Copy link
Member

commented Sep 19, 2018

utACK fa910e4

@practicalswift

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

utACK fa910e4

@laanwj

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

-rpcssl, wow, that one has been gone for a while…

ACK

@scravy

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2018

utACK fa910e4

1 similar comment
@fanquake

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

utACK fa910e4

@MarcoFalke MarcoFalke merged commit fa910e4 into bitcoin:master Sep 20, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Sep 20, 2018

Merge #14272: init: Remove deprecated args from hidden args
fa910e4 init: Remove deprecated args from hidden args (MarcoFalke)

Pull request description:

  The args have been deprecated since 0.17 (maybe longer) and since we reject unknown args, there is no need to add deprecated args to the list of hidden args and then hand-craft an error message if a user provides them.

Tree-SHA512: 3a3191439ab0d7969fb72801d097bd86998524f84b3819380224f746cbe4b0f57beec1ad34744424f6587038035b0ddf418ad13171a8d9c3b97b4f3b7b3222a3

@MarcoFalke MarcoFalke deleted the MarcoFalke:Mf1809-initDeprecatedArgs branch Sep 20, 2018

@promag

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

ACK fa910e4, verified all occurrences of these deprecated args were removed.

@jnewbery

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

I think you can also remove -usehd, since it doesn't have any effect from 0.16 onwards.

@MarcoFalke

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2018

@jnewbery It is currently used to inform the user of "You can't enable/disable HD on an already existing non-HD/HD wallet". I think it gives users a feeling of assurance that their existing wallet file is indeed hd or non-hd.

@MarcoFalke

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2018

I'll leave the decision whether this can be removed to the wallet-people.

@jnewbery

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

wallet-person
paging wallet-people

@jnewbery jnewbery referenced this pull request Sep 20, 2018

Merged

[wallet] Remove -usehd #14282

MarcoFalke added a commit that referenced this pull request Sep 26, 2018

Merge #14282: [wallet] Remove -usehd
7ac911a [docs] Add release notes for removing `-usehd` (John Newbery)
25548b2 [wallet] Remove -usehd (John Newbery)

Pull request description:

  `-usehd` is no longer used (except to tell the user that they've set it incorrectly for the wallet that they're loading). Remove it (in the same spirit as #14272)

Tree-SHA512: 5bdcd2bb9bb8504a01343595bcd1bd433d97b730255152c725103c1ac3fa3a9d9e5220a4c29d4c72307cf803e1c09d31080f83603c23dc77263846e17b1826f0
// Check for -socks - as this is a privacy risk to continue, exit here
if (gArgs.IsArgSet("-socks"))
return InitError(_("Unsupported argument -socks found. Setting SOCKS version isn't possible anymore, only SOCKS5 proxies are supported."));
// Check for -tor - as this is a privacy risk to continue, exit here

This comment has been minimized.

Copy link
@luke-jr

luke-jr Dec 20, 2018

Member

Won't this just yield a warning now?

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Dec 20, 2018

Author Member

Not that I am aware of

This comment has been minimized.

Copy link
@luke-jr

luke-jr Dec 20, 2018

Member

If the user has tor=1 in bitcoin.conf, where will the InitError come from?

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Dec 20, 2018

Author Member

Oh, I was testing src/qt/bitcoin-qt -notor, which gave me and still gives

screenshot from 2018-12-20 20-07-05

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.