Skip to content

Conversation

@glozow
Copy link
Member

@glozow glozow commented Aug 11, 2022

These values are both intended for the same thing. Their divergence seems to be the result of an incomplete rename.

These values are both intended for the same thing. Their divergence
seems to be the result of an incomplete rename.
@achow101
Copy link
Member

ACK acda7e8

Copy link
Member

@furszy furszy left a comment

Choose a reason for hiding this comment

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

ACK acda7e8

was asking the same in #25647 (comment)

@glozow
Copy link
Member Author

glozow commented Aug 11, 2022

(Credit to @S3RK for pointing this out to me)

@murchandamus
Copy link
Contributor

ACK acda7e8

I'm surprised this didn't get caught by tests. Perhaps we are missing one for the minimum change. My gut feeling is that it wouldn't be trivial to test, so that might be out of scope for this PR, though.

Copy link
Contributor

@aureleoules aureleoules left a comment

Choose a reason for hiding this comment

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

ACK acda7e8.
Verified m_change_target is unused and m_min_change_target is the variable that should be used.

@achow101 achow101 merged commit 5d294bc into bitcoin:master Aug 11, 2022
@glozow glozow deleted the 2022-08-change-targets branch August 12, 2022 09:00
@S3RK
Copy link
Contributor

S3RK commented Aug 12, 2022

Post merge ACK acda7e8.

@bitcoin bitcoin deleted a comment from Deadyourface Jan 1, 2023
@bitcoin bitcoin locked and limited conversation to collaborators Jan 1, 2024
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.

6 participants