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

Show changes in BSQ Supply over time #6106

Merged
merged 5 commits into from May 10, 2022
Merged

Show changes in BSQ Supply over time #6106

merged 5 commits into from May 10, 2022

Conversation

ghost
Copy link

@ghost ghost commented Mar 15, 2022

Fixes #6100

total-bsq-supply

Note: values are generated by this script and hardcoded in app.

@ghost ghost marked this pull request as ready for review March 15, 2022 21:00
@pazza83
Copy link

pazza83 commented Mar 16, 2022

Hi @xyzmaker123 thanks for doing this, looks great.

Can you post a screenshot of what it would look like when 'Month' is selected.

Many thanks

@ghost
Copy link
Author

ghost commented Mar 16, 2022

Hi @pazza83 - this is screenshot with 'Month' selected:

bsq-supply-month

@pazza83
Copy link

pazza83 commented Mar 16, 2022

Thanks, looks great to me

.collect(Collectors.toList());

items.forEach(i -> {
log.info("" + i.getKey());
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why you are logging this?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, it's leftover - I'll remove it. Thanks for your vigilance.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't force-push when in review, just make another commit. Otherwise it is harder to update the branch and to look at the changes.

Copy link
Author

Choose a reason for hiding this comment

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

OK - my mistake with leaving this logging code was so pitiful that I just wanted to clear it from git history :)

@ghost ghost requested a review from ripcurlx March 17, 2022 16:21
@ripcurlx
Copy link
Contributor

I don't t hink it is a good idea to also hardcode the values post-DAO as well. Doing it like this someone needs to update the values for each release (we have this issue already on the website, where it is stuck at cycle 29 - @m52go is there someone who is aware of doing this instead of you?). As total issued and total burned BSQ are already calculated for this graph it would be better IMO to use those than to hardcode the values. Also it would make the graph more correct post-DAO as ATM the total cycle values (not monthly) are shown, no matter if yearly, monthly or daily is selected.

Bildschirmfoto 2022-03-21 um 12 38 19
Bildschirmfoto 2022-03-21 um 12 38 24
Bildschirmfoto 2022-03-21 um 12 38 31
Bildschirmfoto 2022-03-21 um 12 38 44

@ghost
Copy link
Author

ghost commented Mar 21, 2022

Thanks for feedback @ripcurlx - I'll try to update pull request with dynamic calculation of BSQ supply.

@ghost
Copy link
Author

ghost commented Mar 22, 2022

@ripcurlx I just pushed commit with dynamic calculation of chart data

@ghost
Copy link
Author

ghost commented Apr 27, 2022

@ripcurlx Review? 🙏

@ripcurlx
Copy link
Contributor

ripcurlx commented May 5, 2022

@ripcurlx Review? 🙏

I'll have a look at it soon 😄

@ripcurlx
Copy link
Contributor

Rebased the PR and fixed two issues.

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 it on Mainnet and changes are looking fine

@ripcurlx ripcurlx added this to the v1.9.2 milestone May 10, 2022
@ripcurlx ripcurlx merged commit cc252f6 into bisq-network:master May 10, 2022
@ghost ghost mentioned this pull request May 11, 2022
@ghost ghost mentioned this pull request Jun 26, 2022
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.

Show changes in BSQ Supply over time in app
2 participants