Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Aug 31, 2021

It would be confusing to keep unused and dead code.

@DrahtBot DrahtBot added the Tests label Aug 31, 2021
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK fa3bd9d.

PR #22848 is related to this one. Without the binary serialization this PR removes, the JSON serialization which #22848 exposes is even more necessary for gui/node process separation. Either binary or json serialization for banmap_t will work for that, as long as there is some publicly exposed banmap<->string serialization function.

@theStack
Copy link
Contributor

Concept ACK

@Zero-1729
Copy link
Contributor

Approach ACK

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-review ACK fa3bd9d

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 1, 2021

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #22848 (MOVEONLY: Expose BanMapToJson / BanMapFromJson by ryanofsky)
  • #22762 (Raise InitError when peers.dat is invalid or corrupted by MarcoFalke)
  • #10102 ([experimental] Multiprocess bitcoin by ryanofsky)

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.

@maflcko maflcko merged commit b3a2b8c into bitcoin:master Sep 1, 2021
@maflcko maflcko deleted the 2109-remBanEntrySer branch September 1, 2021 07:25
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 1, 2021
fa3bd9d Remove CBanEntry::SetNull (MarcoFalke)
fab53ff Remove unused SERIALIZE_METHODS for CBanEntry (MarcoFalke)

Pull request description:

  It would be confusing to keep unused and dead code.

ACKs for top commit:
  ryanofsky:
    Code review ACK fa3bd9d.
  theStack:
    Code-review ACK fa3bd9d

Tree-SHA512: 85ab8de2ad1ada08e745806f2992def08bf8ead268caed7700a9fc61e3c7646e4ed7ae50a6d591c5bb9467f8999ea063ce5b5bd4fa0d58d8fc9d89e5a91f35a5
@DrahtBot DrahtBot mentioned this pull request Sep 1, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 1, 2022
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.

5 participants