util: remove unwanted fields from bitcoin-cli -getinfo by malevolent · Pull Request #17650 · bitcoin/bitcoin

malevolent · 2019-12-02T09:40:56Z

Removed the following fields from -getinfo: protocolversion, walletversion and keypoololdest.

laanwj · 2019-12-02T10:30:56Z

Concept ACK

Code changes look good to me. Some git nits:

Please adapt your commit message to the standard format: a short title, an empty line then the body of the text
Please set a username and mail address other than "EC2 Default User"

practicalswift · 2019-12-02T10:39:59Z

Concept ACK

@malevolent Welcome as a contributor! :)

cvengler · 2019-12-02T16:26:44Z

Concept ACK

Your tests are failing.
Please update test/functional/interface_bitcoin_cli.py
See line 61-76

malevolent · 2019-12-03T00:40:27Z

@laanwj Yes sir, done and done.
@practicalswift Thank you :)
@emilengler Thanks, just pushed change to delete tests for removed api calls.

fanquake · 2019-12-03T00:44:30Z

Please squash your commits.

Please adapt your commit message to the standard format: a short title, an empty line then the body of the text

You could use a commit title like util: remove unwanted fields from bitcoin-cli -getinfo.

malevolent · 2019-12-03T12:43:14Z

Hi @fanquake , thanks for letting me know, I rebased and used better titles for the commit messages. Please let me know if theres anything else needed

practicalswift · 2019-12-03T13:30:48Z

ACK 868a32228688afca262f10d3be9fa6caa9cdea65 -- diff looks correct

laanwj · 2019-12-03T15:21:42Z

the author is still reported as "EC2 Default User"

commit 868a32228688afca262f10d3be9fa6caa9cdea65
Author: EC2 Default User <[email protected]>

i think you need to do git commit --amend --reset-author to fix this, if you want to be credited correctly

In accordance with bitcoin#17314, Removing noisy fields from -getinfo. Fields removed: protocolversion, walletversion and keypoololdest. In addition to changing bitcoin-cli -getinfo, there is another change to test/functional/interface_bitcoin_cli.py. This change deletes tests that utilize removed -getinfo calls.

malevolent · 2019-12-04T06:50:12Z

@laanwj Thanks I think that fixed it, let me know if there is anything else I need to do

laanwj · 2019-12-04T10:05:58Z

ACK 01c8701

practicalswift · 2019-12-04T10:15:35Z

ACK 01c8701 -- diff looks correct

malevolent · 2019-12-04T19:30:49Z

@laanwj is there anything I need to do for the release note label that was added? Also, I think I should change it from saying "closes #17314" since there are other parts to that issue, right?

fanquake · 2019-12-04T19:32:15Z

release note label that was added?

This can be addressed at a later date.

I think I should change itfrom saying "closes #17314" since there are other parts to that issue, right?

Correct, that issue doesn't need to be closed after this is merged.

achow101 · 2019-12-04T19:55:09Z

ACK 01c8701

01c8701 util: remove unwanted fields from bitcoin-cli -getinfo (malevolent) Pull request description: Removed the following fields from -getinfo: protocolversion, walletversion and keypoololdest. This change closes #17314 . ACKs for top commit: laanwj: ACK 01c8701 achow101: ACK 01c8701 practicalswift: ACK 01c8701 -- diff looks correct Tree-SHA512: c98f2e8a3fee04d46766f70cb88f4e49e892a4424eff8940a7d48e9e808597b702427225788f984f5c3641591fd8d86acee56774afde1d57a4259c31d971ea08

…etinfo 01c8701 util: remove unwanted fields from bitcoin-cli -getinfo (malevolent) Pull request description: Removed the following fields from -getinfo: protocolversion, walletversion and keypoololdest. This change closes bitcoin#17314 . ACKs for top commit: laanwj: ACK 01c8701 achow101: ACK 01c8701 practicalswift: ACK 01c8701 -- diff looks correct Tree-SHA512: c98f2e8a3fee04d46766f70cb88f4e49e892a4424eff8940a7d48e9e808597b702427225788f984f5c3641591fd8d86acee56774afde1d57a4259c31d971ea08

laanwj · 2019-12-05T09:28:57Z

release note label that was added?

Yea, it's probably better to wait until the release in this case, if there's other bitcoin-cli changes at well they should be grouped and not scattered over the place.

fanquake · 2020-05-23T07:31:10Z

@laanwj Could you add any relevant release notes to https://github.com/bitcoin-core/bitcoin-devwiki/wiki/0.20.0-Release-Notes-Draft ?

maflcko · 2020-05-23T11:32:12Z

Done https://github.com/bitcoin-core/bitcoin-devwiki/wiki/0.20.0-Release-Notes-Draft/_compare/b03541ec204bd2c7460b3c383c45f84b21c5b65c...3f531649bb00d9824814ff3a7e9c7282e806844b

Summary: > In accordance with #17314, Removing noisy fields from -getinfo. Fields removed: protocolversion, walletversion and keypoololdest. In addition to changing bitcoin-cli -getinfo, there is another change to test/functional/interface_bitcoin_cli.py. This change deletes tests that utilize removed -getinfo calls. Add a release note about this and also about new fields introduced in D8220/[[bitcoin/bitcoin#17302 | PR17302]]. This is extracted from Core `release-notes-0.20.0.md` This is a backport of Core [[bitcoin/bitcoin#17650 | PR17650]] Test Plan: `src/bitcoin-cli -getinfo` `test/functional/test_runner.py interface_bitcoin_cli.py` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D8300

…etinfo 01c8701 util: remove unwanted fields from bitcoin-cli -getinfo (malevolent) Pull request description: Removed the following fields from -getinfo: protocolversion, walletversion and keypoololdest. This change closes bitcoin#17314 . ACKs for top commit: laanwj: ACK 01c8701 achow101: ACK 01c8701 practicalswift: ACK 01c8701 -- diff looks correct Tree-SHA512: c98f2e8a3fee04d46766f70cb88f4e49e892a4424eff8940a7d48e9e808597b702427225788f984f5c3641591fd8d86acee56774afde1d57a4259c31d971ea08

malevolent changed the title ~~Removed unwanted fields from -getinfo. Closes #17314~~ Removed unwanted fields from -getinfo. Dec 2, 2019

fanquake added Utils/log/libs Waiting for author labels Dec 2, 2019

fanquake changed the title ~~Removed unwanted fields from -getinfo.~~ util: remove unwanted fields from bitcoin-cli -getinfo Dec 3, 2019

malevolent force-pushed the rethink_bitcoin_cli branch from ad60f38 to 868a322 Compare December 3, 2019 12:52

fanquake removed the Waiting for author label Dec 3, 2019

malevolent force-pushed the rethink_bitcoin_cli branch from 868a322 to 01c8701 Compare December 4, 2019 06:49

laanwj mentioned this pull request Dec 4, 2019

Re-thinking bitcoin-cli -getinfo #17314

Closed

laanwj added the Needs release note label Dec 4, 2019

fanquake merged commit 01c8701 into bitcoin:master Dec 4, 2019

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

fanquake removed the Needs release note label May 23, 2020

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Conversation

malevolent commented Dec 2, 2019 • edited by fanquake Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Dec 2, 2019

Uh oh!

practicalswift commented Dec 2, 2019

Uh oh!

cvengler commented Dec 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

malevolent commented Dec 3, 2019

Uh oh!

fanquake commented Dec 3, 2019

Uh oh!

malevolent commented Dec 3, 2019

Uh oh!

practicalswift commented Dec 3, 2019

Uh oh!

laanwj commented Dec 3, 2019

Uh oh!

malevolent commented Dec 4, 2019

Uh oh!

laanwj commented Dec 4, 2019

Uh oh!

practicalswift commented Dec 4, 2019

Uh oh!

malevolent commented Dec 4, 2019

Uh oh!

fanquake commented Dec 4, 2019

Uh oh!

achow101 commented Dec 4, 2019

Uh oh!

laanwj commented Dec 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanquake commented May 23, 2020

Uh oh!

maflcko commented May 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

malevolent commented Dec 2, 2019 •

edited by fanquake

Loading

cvengler commented Dec 2, 2019 •

edited

Loading

laanwj commented Dec 5, 2019 •

edited

Loading