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

Display 2 decimal places for fiat trading fees #3973

Merged
merged 1 commit into from Feb 21, 2020

Conversation

@milesap
Copy link
Contributor

milesap commented Feb 15, 2020

Fixes #3972

Fixes an issue where fiat trading fees were being rounded to the nearest whole dollar.

Tests pass, as did my visual sanity checks:

3972-0
3972-1

Copy link
Contributor

stejbac left a comment

To avoid duplication, it might be better to use DisplayUtils.formatAverageVolumeWithCode. (I added that method specifically to restore the original behaviour of formatVolumeWithCode where needed.)

Copy link
Member

ripcurlx left a comment

NACK - please see #3973 (review)

@ripcurlx

This comment has been minimized.

Copy link
Member

ripcurlx commented Feb 20, 2020

@milesap Did you see #3973 (review)?

@milesap milesap force-pushed the milesap:3972-tradefee branch from d3bf56a to 372e78c Feb 20, 2020
@milesap

This comment has been minimized.

Copy link
Contributor Author

milesap commented Feb 20, 2020

Sorry got distracted, pushed updated code per the review. @ripcurlx

@milesap milesap requested review from stejbac and ripcurlx Feb 20, 2020
@ripcurlx

This comment has been minimized.

Copy link
Member

ripcurlx commented Feb 21, 2020

ACK

Tested it on Mainnet and it works again as in v1.2.5-

@ripcurlx ripcurlx merged commit adafd40 into bisq-network:master Feb 21, 2020
2 checks passed
2 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ripcurlx ripcurlx added this to the v1.2.8 milestone Feb 26, 2020
@ripcurlx ripcurlx mentioned this pull request Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.