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

Fixes for issues #2741, #2944, #2955 #2976

Merged
merged 8 commits into from Aug 9, 2019

Conversation

@niyid
Copy link
Contributor

commented Jul 18, 2019

  1. Issue #2944 is a bug. The low and high price values were inverted when the currency type is cryptocurrency.

  2. Issue #2955 is a feature add - inclusion of median statistic for display in the candle tool-tip.

  3. Issue #2741 is a simple change in i18n message bundle for better clarity.

fixes #2741, fixes #2955, fixes #2944

@niyid niyid requested review from ripcurlx and sqrrm as code owners Jul 18, 2019
@niyid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

Hi Christopher,

I think I finally got it right this time. Sorry for the inconvenience and thanks for your patience.

Regards.

@niyid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

Should median computation function be added to bisq.common.util.MathUtils rather than locally to bisq.desktop.main.market.trades.TradesChartsViewModel as Long findMedian(Long[] prices)?

Copy link
Member

left a comment

NACK - please see my translation comment

niyid added 2 commits Jul 23, 2019
@ripcurlx

This comment has been minimized.

Copy link
Member

commented Jul 24, 2019

@niyid For your next PRs please make one PR per issue, so the PRs are as small as possible. Thanks!

@niyid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2019

Copy link
Contributor Author

left a comment

Hi @ripcurlx. Can you please reject this latest commit to that I can push on a separate PR? Apologies.

In the interim I have renamed the pull request in order to be able to make sense out of it.

@niyid niyid changed the title Fixes to issues 2944 and 2955 Fixes for issues 1896, 2944, 2955 Jul 24, 2019
@ripcurlx

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

Hi @ripcurlx. Can you please reject this latest commit to that I can push on a separate PR? Apologies.

@niyid Do you mean I should close this PR? I don't know what you mean with rejecting a commit.

@niyid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2019

Hi @ripcurlx. Can you please reject this latest commit to that I can push on a separate PR? Apologies.

@niyid Do you mean I should close this PR? I don't know what you mean with rejecting a commit.

Sorry about the choice of words. I had mistakenly pushed a commit for #1896 on this same PR. I then created a separate PR for #1896 after realizing this mistake.

@ripcurlx ripcurlx changed the title Fixes for issues 1896, 2944, 2955 Fixes for issues 2944, 2955 Jul 31, 2019
@ripcurlx

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

@niyid I just wanted to test the PR but saw that it has a a conflict in Namecoin.java. As soon as this is fixed, I'll test the PR.

@ripcurlx

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

Also the tests for the coins are failing for me. Somehow Travis didn't run for this PR. I just tried to run it manually again.

@niyid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2019

@ripcurlx

Yes. The last commit should be deleted. Those changes (which were incomplete) are now in the completed PR for the fix to #1896.

@niyid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2019

This reverts commit 363f2e9.
@niyid niyid changed the title Fixes for issues 2944, 2955 Fixes for issues #2741, #2944, #2955 Jul 31, 2019
@niyid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2019

@ripcurlx

Yes. The last commit should be deleted. Those changes (which were incomplete) are now in the completed PR for the fix to #1896.

I have reverted the last commit. Please go ahead with the PR test. Apologies for any inconvenience.

Copy link
Member

left a comment

Ack

@freimair freimair referenced this pull request Aug 9, 2019
@sqrrm sqrrm merged commit fb1c96d into bisq-network:master Aug 9, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ripcurlx ripcurlx referenced this pull request Sep 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.