RPC: Chainparams: Remove Chainparams::fTestnetToBeDeprecatedFieldRPC #8921

pull jtimon wants to merge 1 commits into bitcoin:master from jtimon:0.13-rpc-chain changing 5 files +3 −9

jtimon commented at 11:04 AM on October 14, 2016: contributor

Deprecate the "testnet" field in some info RPC calls in favor of the more generic "chain".

Unfortunately #4333 didn't add a "chain" field for rpcmic::getinfo like it did for rpcmining::getmininginfo. fTestnetToBeDeprecatedFieldRPC was created in #4802.
MarcoFalke commented at 11:14 AM on October 14, 2016: member

This needs mention in the release notes. Even though this was deprecated internally a while ago, users may not have noticed... What about keeping both (the old "testnet" and the new "chain") for a while and mark the "testnet" one as deprecated in the help texts? Then remove "testnet" in 0.15
jtimon commented at 11:30 AM on October 14, 2016: contributor

@MarcoFalke yes, that (having both) was done for rpcmining::getmininginfo but not for rpcmic::getinfo (see #4333 (comment) ). IIRC we mentioned in the release notes at the time that testnet would be eventually removed in favor of chain. Maybe @Diapolo (who coded #4326) remembers better? At least for rpcmining::getmininginfo I think we're safe taking it out for 0.14. rpcmic::getinfo hasn't died yet, but since @laanwj said that was the plan, I don't think that should be a blocker.

EDIT: Updated release notes.
jtimon force-pushed on Oct 14, 2016
fanquake added the label RPC/REST/ZMQ on Oct 14, 2016
laanwj commented at 7:29 AM on October 15, 2016: member

Please, do not make changes to getinfo(). It's going to be removed at some point (now that it's been deprecated), let's not burden anyone with API changes on it until then. at that point you can freely remove this method. There's no hurry.
MarcoFalke commented at 7:32 AM on October 15, 2016: member

Agree, but you could edit the help text of getinfo to tell where the replacement can be found.
laanwj commented at 7:35 AM on October 15, 2016: member

Changing the help text of getinfo to point to the new field on getblockchaininfo, would be great.

We could even include this table: #8455 (comment) (which I've programmatically verified)
jtimon force-pushed on Oct 18, 2016
jtimon commented at 10:22 AM on October 18, 2016: contributor

Updated without touching "getinfo" given that is to be removed for 0.15. I dislike leaving Params().NetworkIDString() == CBaseChainParams::TESTNET there but since it is only one place that is going to be removed, I think it's fine.
MarcoFalke commented at 10:32 AM on October 18, 2016: member

utACK 02eeffac5f686ae59be3db79761e55c12b29d9c9

in doc/release-notes.md:None in 02eeffac5f outdated

  64 | @@ -65,6 +65,8 @@ contain invalid parameters.
  65 |  
  66 |  The first boolean argument to `getaddednodeinfo` has been removed. This is an incompatible change.
  67 |  
  68 | +Call "getmininginfo" loses the "testnet" field in favor of the more generic "chain" (which has been having for years).

laanwj commented at 11:11 AM on October 18, 2016:

"which has been having" I'd reword that as "which has been present"

laanwj commented at 11:12 AM on October 18, 2016: member

utACK - language nit
RPC: Chainparams: Remove Chainparams::fTestnetToBeDeprecatedFieldRPC b26a7b5377
jtimon force-pushed on Oct 18, 2016
jtimon commented at 11:18 AM on October 18, 2016: contributor

Updated with release notes corrected.
laanwj added the label Refactoring on Oct 18, 2016
laanwj merged this on Oct 18, 2016
laanwj closed this on Oct 18, 2016
laanwj referenced this in commit dd07c6b2cc on Oct 18, 2016
codablock referenced this in commit ce6fbbdd3c on Sep 19, 2017
codablock referenced this in commit c4faa32181 on Jan 12, 2018
andvgal referenced this in commit 2206e06382 on Jan 6, 2019
DrahtBot locked this on Sep 8, 2021

Contributors

jtimon

MarcoFalke

laanwj

Labels

Refactoring RPC/REST/ZMQ

Linked (view graph)

#4333 Replace std::string PaymentServer::mapNetworkIdToName and CChainParams::...#4802 Remove CChainParams::NetworkID()