Skip to content

Qt: move paymentACK handling to paymentserver#3130

Merged
laanwj merged 1 commit intobitcoin:masterfrom
Diapolo:paymentACK_via_message
Oct 25, 2013
Merged

Qt: move paymentACK handling to paymentserver#3130
laanwj merged 1 commit intobitcoin:masterfrom
Diapolo:paymentACK_via_message

Conversation

@Diapolo
Copy link
Copy Markdown

@Diapolo Diapolo commented Oct 22, 2013

  • add new slot handlePaymentACK() to paymentserver, which handles
    paymentACK messages (currently we just display them)
  • make paymentACK message a modal information dialog
  • change some QObject::tr() to just tr()
  • clarify the processPaymentRequest() error, when IsDust()
  • small string change to prevent a tripple + usage with QString

@laanwj
Copy link
Copy Markdown
Member

laanwj commented Oct 23, 2013

Haven't looked at it in detail yet, but moving the message to PaymentServer makes sense as it only re-emits a message.

@laanwj
Copy link
Copy Markdown
Member

laanwj commented Oct 24, 2013

I don't think we should make the paymentack message a tray message.
As we are not storing it anywhere for later reference at the moment (or are we?), it's good to give the user some time to read it or even copy it.

@Diapolo
Copy link
Copy Markdown
Author

Diapolo commented Oct 24, 2013

@laanwj That's fine with me... perhaps there are better methods for handling this message, but that's out of the score of this pr.

@Diapolo
Copy link
Copy Markdown
Author

Diapolo commented Oct 24, 2013

Updated:

  • make paymentACK message a modal information dialog

Comment thread src/qt/paymentserver.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm I'd suggest to keep the signal, and subscribe to it locally; what if we want to do something else with the ACK later on?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm going to change that :).

- add new slot handlePaymentACK() to paymentserver, which handles
  paymentACK messages (currently we just display them)
- make paymentACK message a modal information dialog
- change some QObject::tr() to just tr()
- clarify the processPaymentRequest() error, when IsDust()
- small string change to prevent a tripple + usage with QString
@Diapolo
Copy link
Copy Markdown
Author

Diapolo commented Oct 24, 2013

Updated to reflect @laanwj's suggestion:

  • add new slot handlePaymentACK() to paymentserver, which handles paymentACK messages (currently we just display them)

@laanwj
Copy link
Copy Markdown
Member

laanwj commented Oct 24, 2013

ACK

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/08dd1b7be172f7ea580dc9bdf20ac15ea9a2ed31 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

laanwj added a commit that referenced this pull request Oct 25, 2013
Qt: move paymentACK handling to paymentserver
@laanwj laanwj merged commit 48cc4fc into bitcoin:master Oct 25, 2013
Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Apr 8, 2020
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants