Skip to content

Conversation

luke-jr
Copy link
Member

@luke-jr luke-jr commented Aug 20, 2020

Refactor split out of #17167

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 21, 2020

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

Conflicts

No conflicts as of last run.

@practicalswift
Copy link
Contributor

Concept ACK: enum class is strictly better

Copy link
Contributor

@promag promag 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 8727019, just needs to format correctly apparently.

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

ACK

@practicalswift
Copy link
Contributor

ACK 40bc8d7: patch looks correct

@practicalswift
Copy link
Contributor

cr ACK c77de62: patch looks correct & enum class is strictly better

@maflcko maflcko merged commit 8c049fe into bitcoin:master Mar 7, 2021
laanwj added a commit that referenced this pull request May 19, 2021
7075f60 scripted-diff: update noban documentation in net_processing.cpp (Jon Atack)
a95540c scripted-diff: rename NetPermissionFlags enumerators (Jon Atack)
810d092 p2p, refactor: make NetPermissionFlags a uint32 enum class (Jon Atack)
7b55a94 p2p: NetPermissions::HasFlag() pass flags param by value (Jon Atack)
91f6e6e scripted-diff: add NetPermissionFlags scopes where not already present (Jon Atack)

Pull request description:

  While reviewing #20196, I noticed the `NetPermissionFlags` enums are frequently called as if they were scoped, yet are still global. This patch upgrades `NetPermissionFlags` to a scoped class enum and updates the enumerator naming, similarly to #19771. See https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum-enumerations for more info.

  This change would eliminate the class of bugs like #20196 (comment) and #21644, as only defined operations on the flags would compile.

ACKs for top commit:
  laanwj:
    Code review ACK 7075f60
  vasild:
    ACK 7075f60

Tree-SHA512: 7fcea66ee499f059efc78c934b5f729b3c8573fe304dee2c27c837c2f662b89324790568246d75b2a574cf9f059b42d3551d928996862f4358055eb43521e6f4
@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.

6 participants