Skip to content
This repository has been archived by the owner on Jun 17, 2020. It is now read-only.

Prune TradeStatisticsManager#printAllCurrencyStats #54

Merged

Conversation

cbeams
Copy link
Contributor

@cbeams cbeams commented Apr 5, 2018

This appears to be dead code, see @blabno's inquiry about it at
https://bisq.slack.com/archives/C1267HFD1/p1521801524000052.

If this is in fact not dead code, i.e. it gets used to print out
certain reports at release boundaries, I suggest we still kill it here,
and re-write that code in such a way that it (a) uses the new bisq.asset
infrastructure and (b) doesn't require duplicating ticker symbols in
this class.

This work was intended for inclusion in #32.

This appears to be dead code, see @blabno's inquiry about it at
https://bisq.slack.com/archives/C1267HFD1/p1521801524000052.

If this is in fact *not* dead code, i.e. it gets used to print out
certain reports at release boundaries, I suggest we still kill it here,
and re-write that code in such a way that it (a) uses the new bisq.asset
infrastructure and (b) doesn't require duplicating ticker symbols in
this class.

This work was intended for inclusion in bisq-network#32.
@ManfredKarrer
Copy link
Contributor

We could consider to automatially de-list assets which have not been traded in a certain period. The delisting would happen first just be excluding them in the displayed list, so users cannot select them anymore. To get a report which assets got delisted to remove code would be good as well.

@cbeams
Copy link
Contributor Author

cbeams commented Apr 5, 2018

That would require that we start tracking a listingDate property in each Asset (which would be fine).

Otherwise, is this an ACK, Manfred? Please just merge if you're good with the change.

Copy link
Contributor

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

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

utACK

@ManfredKarrer ManfredKarrer merged commit 5fffd9e into bisq-network:master Apr 5, 2018
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.

2 participants