Skip to content

Conversation

jonatack
Copy link
Member

This is a migration of bitcoin-core/gui#162 to this repo. It adds a network column to the Peers window, adds the network to the peer details, and renames two peers column headers from NodeId and Node/Service to Peer Id and Address. The initial version of the pull only changed gui code; however, the first commit now touches code in net.{h, cpp} and rpc/net.cpp. Please see the original pull for the review up to this point, including an ACK by hebasto.

Screenshot from 2020-12-27 14-45-31

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@Sjors
Copy link
Member

Sjors commented Dec 28, 2020

Rather than moving the entire PR over, why not limit this PR to the non-GUI changes you need?

@jonatack
Copy link
Member Author

The changes make more sense together (I suspect no one would review the first commit on its own) and it's fewer pulls to maintain and review.

@maflcko
Copy link
Member

maflcko commented Dec 28, 2020

Tend to agree with @Sjors . Most reviewers in this repo can't review the gui changes and a simple RPC refactor can be standalone. Linking to the gui repo for motivation should be sufficient, if it is not motivated by itself enough.

@jonatack
Copy link
Member Author

This pull follows the guidelines in CONTRIBUTING.md about tranversal changes. It is the second pull I have opened for this one change; the first pull originally touched only GUI code, but reviewers asked for transversal changes. Now people are asking for more pulls, with one in one repo gated by the other in the other repo. I understand the reasons for splitting the repos, but it is fair to say those reasons did not include complicating contributions in this way to the GUI.

Up for grabs.

@jonatack jonatack closed this Dec 28, 2020
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants