Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Simplify SendCoinsDialog::updateCoinControlState #284

Merged
merged 1 commit into from
Apr 25, 2021

Conversation

promag
Copy link
Contributor

@promag promag commented Apr 18, 2021

This PR doesn't change behaviour, removes the coin control argument from updateCoinControlState since it's a class member.

@hebasto hebasto changed the title refactor, qt: Simplify SendCoinsDialog::updateCoinControlState refactor: Simplify SendCoinsDialog::updateCoinControlState Apr 19, 2021
@hebasto
Copy link
Member

hebasto commented Apr 20, 2021

Concept ACK on simplifying code.

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.

ACK 5f438d6, I have reviewed the code and it looks OK, I agree it can be merged.

I see this change as a nice bitcoin/bitcoin#18894 follow up.

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

Nice simplification.

Code review ACK 5f438d6

Copy link
Contributor

@kristapsk kristapsk left a comment

Choose a reason for hiding this comment

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

utACK 5f438d6. Code looks correct.

@hebasto hebasto merged commit c4571a0 into bitcoin-core:master Apr 25, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 25, 2021
…eCoinControlState

5f438d6 refactor, qt: Simplify SendCoinsDialog::updateCoinControlState (João Barbosa)

Pull request description:

  This PR doesn't change behaviour, removes the coin control argument from `updateCoinControlState` since it's a class member.

ACKs for top commit:
  hebasto:
    ACK 5f438d6, I have reviewed the code and it looks OK, I agree it can be merged.
  jonatack:
    Code review ACK 5f438d6
  kristapsk:
    utACK 5f438d6. Code looks correct.

Tree-SHA512: 14abaa3d561f8c8854fed989b6aca886dcca42135880bac76070043f61c0042ec8967f2b83e50bbbb82050ef0f074209e97fa300cb4dc51ee182316e0846506d
@promag promag deleted the 2021-04-sendcoinsdialog branch April 26, 2021 07:12
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@bitcoin-core bitcoin-core 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.

4 participants