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

Enforce inclusion of TradeLimits for headless apps #5857

Merged
merged 2 commits into from
Nov 24, 2021

Conversation

chimp1984
Copy link
Contributor

PaymentMethod use an instance of TradeLimits and expect that it
has been injected, which is the case for desktop but not for
headless apps, so we enforce injection in the app base classes
used for headless apps.

The validation of trade statistics use a method in PaymentMethod where that
dependency is required.

Tha hack how the PaymentMethod use TradeLimits is not nice, but
would require more effort for refactoring.

ripcurlx
ripcurlx previously approved these changes Nov 23, 2021
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

ACK - Tested on Regtest.

@chimp1984
Copy link
Contributor Author

@ripcurlx Please ignore the Codacy complaints.

PaymentMethod use an instance of TradeLimits and expect that it
has been injected, which is the case for desktop but not for
headless apps, so we enforce injection in the app base classes
used for headless apps.

The validation of trade statistics use a method in PaymentMethod
where that dependency is required.

Tha hack how the PaymentMethod use TradeLimits is not nice, but
would require more effort for refactoring.
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK

@ripcurlx ripcurlx added this to the v1.8.0 milestone Nov 24, 2021
@ripcurlx ripcurlx merged commit 3570750 into bisq-network:master Nov 24, 2021
ripcurlx added a commit to ripcurlx/bisq-desktop that referenced this pull request Nov 24, 2021
@chimp1984 chimp1984 deleted the add-missing-dependency branch November 24, 2021 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants