Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Mar 5, 2021

This removes a bunch of boilerplate, makes the code easier to read. Also, C++11 member initialization avoids accidental uninitialized members.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

cr ACK fa476f1

@practicalswift
Copy link
Contributor

Safer, more readable and shorter (+18 −35): nice! :)

cr ACK fa476f1

I'd be happy to review any PR migrating our classes to using C++11 default member initialization: safer is better! :)

Copy link
Contributor

@jnewbery jnewbery left a comment

Choose a reason for hiding this comment

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

utACK fa476f1

@fanquake fanquake merged commit e175ca9 into bitcoin:master Mar 9, 2021
@maflcko maflcko deleted the 2103-netCNodeStateRefactor branch March 9, 2021 12:57
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 9, 2021
fa476f1 Use C++11 member initializer in CNodeState (MarcoFalke)

Pull request description:

  This removes a bunch of boilerplate, makes the code easier to read. Also, C++11 member initialization avoids accidental uninitialized members.

ACKs for top commit:
  practicalswift:
    cr ACK fa476f1
  hebasto:
    cr ACK fa476f1
  jnewbery:
    utACK fa476f1

Tree-SHA512: 5c876717d30ded975e29bfbc77804012179588a13f950f0b2ec93fa9dbd5cf6b52fe86414fd5d1cce021db2ec77e271d533b0f7a8d6eeaac0feb9e6dbaec9ff2
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Mar 15, 2021
6927933 [net processing] Add ChainSyncTimeoutState default initializers (John Newbery)
55966e0 [net processing] Remove CNodeState ctor body (John Newbery)

Pull request description:

  This addresses the two outstanding review comments from bitcoin#21370.

ACKs for top commit:
  practicalswift:
    cr ACK 6927933: patch looks correct
  hebasto:
    ACK 6927933, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: b3ef5c8a096e447887df255406b3a760f01c73e2b942374595416b4b4031fc69b89cd93168c45040489d581f340b2a62d3fbabd207d4307f587c00a7a7daacd1
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 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