Skip to content

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Sep 25, 2021

The first removed comment was introduced in #5288, the second one in #13503.

Both are outdated since #14336.

@DrahtBot DrahtBot added the Docs label Sep 25, 2021
Copy link
Contributor

@dunxen dunxen left a comment

Choose a reason for hiding this comment

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

crACK ee7891a

@laanwj laanwj merged commit 8b523f2 into bitcoin:master Sep 27, 2021
@hebasto hebasto deleted the 210925-comment branch September 27, 2021 10:59
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 27, 2021
ee7891a doc: Remove outdated comments (Hennadii Stepanov)

Pull request description:

  The first removed comment was introduced in bitcoin#5288, the second one in bitcoin#13503.

  Both are outdated since bitcoin#14336.

ACKs for top commit:
  duncandean:
    crACK ee7891a

Tree-SHA512: a2d6071919e81c916bfc2178109bbc464417321bcc567ed0644448c5faea8e58cb08a7657afa1b6ffe1fb63e114a2a47b31c893e471839ba9d49a3986e68b2a7
nMaxConnections = std::max(nUserMaxConnections, 0);

// Trim requested connection counts, to fit into system limitations
// <int> in std::min<int>(...) to work around FreeBSD compilation issue described in #2695
Copy link
Member

Choose a reason for hiding this comment

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

Both are outdated since #14336.

Why? The code the comment referred to previously was just moved a few lines down (unchanged) in #14336. This PR should have either moved the comment down, or had an actual explanation of why the FreeBSD compilation issue is no longer relevant.

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦‍♂️

ouch, you are right...

laanwj added a commit to bitcoin-core/gui that referenced this pull request Sep 30, 2021
…nd place comment correctly

8ff3743 Revert "doc: Remove outdated comments" (Hennadii Stepanov)

Pull request description:

  Unfortunately, in #23094 the assumption that #14336 makes comments outdated is wrong. As pointed in  bitcoin/bitcoin#23094 (comment), the #14336 just moved the relevant code a few lines down.

  This PR reverts commit ee7891a, and moves the comments into the right place.

  I apologize about that.

ACKs for top commit:
  MarcoFalke:
    cr ACK 8ff3743
  laanwj:
    ACK 8ff3743

Tree-SHA512: 84aca627bb5b49c06fc172778f9b9407482c5a873ccbc3dc40167e6a8ad0bc60475d6a469c843b7b42712e35cf3fc2d3518923e791d5e0c59628e042acc72747
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 30, 2021
… comment correctly

8ff3743 Revert "doc: Remove outdated comments" (Hennadii Stepanov)

Pull request description:

  Unfortunately, in bitcoin#23094 the assumption that bitcoin#14336 makes comments outdated is wrong. As pointed in  bitcoin#23094 (comment), the bitcoin#14336 just moved the relevant code a few lines down.

  This PR reverts commit ee7891a, and moves the comments into the right place.

  I apologize about that.

ACKs for top commit:
  MarcoFalke:
    cr ACK 8ff3743
  laanwj:
    ACK 8ff3743

Tree-SHA512: 84aca627bb5b49c06fc172778f9b9407482c5a873ccbc3dc40167e6a8ad0bc60475d6a469c843b7b42712e35cf3fc2d3518923e791d5e0c59628e042acc72747
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 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