Skip to content

Conversation

@promag
Copy link
Contributor

@promag promag commented Jul 31, 2019

Added in #14573 but not used, so begone.

@fanquake fanquake added the GUI label Jul 31, 2019
@cvengler
Copy link
Contributor

cvengler commented Jul 31, 2019

Maybe it's better to comment it out so we can use it if may need it in future (If that's a thing in this project).

@promag
Copy link
Contributor Author

promag commented Jul 31, 2019

IMO better is to kill unused code. It's also easy to bring this back, if needed.

@practicalswift
Copy link
Contributor

utACK b078067

@cvengler
Copy link
Contributor

It perfectly removes it, utACK

@promag
Copy link
Contributor Author

promag commented Jul 31, 2019

@practicalswift @emilengler please take a new look at CONTRIBUTING.md#peer-review, it was recently updated in #16149.

@cvengler
Copy link
Contributor

@promag Didn't knew it, then
Concept ACK
Approach ACK

@hebasto
Copy link
Member

hebasto commented Aug 1, 2019

ACK b078067
I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged.

@laanwj
Copy link
Member

laanwj commented Aug 1, 2019

ACK b078067, there's nothing really to test here

Maybe it's better to comment it out so we can use it if may need it in future (If that's a thing in this project).

I think if we don't expect it to be necessary at some point it's better to remove it.
(I'm generally for removing dead code unless it breaks API expectations)

@laanwj laanwj merged commit b078067 into bitcoin:master Aug 1, 2019
laanwj added a commit that referenced this pull request Aug 1, 2019
b078067 gui: Remove unused RPCConsole::tabFocus (João Barbosa)

Pull request description:

  Added in #14573 but not used, so begone.

ACKs for top commit:
  practicalswift:
    utACK b078067
  hebasto:
    ACK b078067
  laanwj:
    ACK b078067, there's nothing really to test here

Tree-SHA512: 237276dea4d174b5fca34855447146f79c3faaae7179f4245c70e2070b49282d95f886b1be6d2a33713c81a254f4483a4e4bf850053a8dcb18a3a897bd3da08e
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 12, 2020
Summary:
Overall this PR does the following:

 - add top level menu Window
 - add Minimize and Zoom actions to Window menu
 - move Sending/Receiving address to Window
 - remove Help->Debug menu and add one menu entry for each debug window tab in the Window menu

The ellipsis in the name of `Sending/Receiving addresses` actions are be removed, according to common practices (https://stackoverflow.com/a/637708).
Ellipsis at the end of the name of menu actions suggest that user input is expected.

Backport of Core [[bitcoin/bitcoin#14573 | PR14573]] and [[bitcoin/bitcoin#16514 | PR16514]]

[[bitcoin/bitcoin#16514 | PR16514]] removes the `RPCConsole::tabFocus` method introduced in the first PR, but never used.

Test Plan:
`ninja && ninja check`

Run `src/qt/bitcoin-qt` and check that all menu actions work.

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D7892
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request Apr 14, 2021
Summary:
Overall this PR does the following:

 - add top level menu Window
 - add Minimize and Zoom actions to Window menu
 - move Sending/Receiving address to Window
 - remove Help->Debug menu and add one menu entry for each debug window tab in the Window menu

The ellipsis in the name of `Sending/Receiving addresses` actions are be removed, according to common practices (https://stackoverflow.com/a/637708).
Ellipsis at the end of the name of menu actions suggest that user input is expected.

Backport of Core [[bitcoin/bitcoin#14573 | PR14573]] and [[bitcoin/bitcoin#16514 | PR16514]]

[[bitcoin/bitcoin#16514 | PR16514]] removes the `RPCConsole::tabFocus` method introduced in the first PR, but never used.

Test Plan:
`ninja && ninja check`

Run `src/qt/bitcoin-qt` and check that all menu actions work.

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D7892
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Nov 30, 2021
b078067 gui: Remove unused RPCConsole::tabFocus (João Barbosa)

Pull request description:

  Added in bitcoin#14573 but not used, so begone.

ACKs for top commit:
  practicalswift:
    utACK b078067
  hebasto:
    ACK b078067
  laanwj:
    ACK b078067, there's nothing really to test here

Tree-SHA512: 237276dea4d174b5fca34855447146f79c3faaae7179f4245c70e2070b49282d95f886b1be6d2a33713c81a254f4483a4e4bf850053a8dcb18a3a897bd3da08e
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants