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

feat: display estimated effective volume during buy (#479) #485

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

LevilkTheReal
Copy link

Issue: #479

Solution:
Screenshot 2023-12-05 at 15 34 12

@Cafe137 FYI

I could not run the tests. I am getting this error. Any idea?
Screenshot 2023-12-05 at 16 34 37

(I've extended the tests with this line tho: expect(consoleMessages[2]).toBe('Estimated effective volume: 41.17 GB'))

Another thing. The Storage class is not that precise. For instance, it converts the incoming effective bytes to 41.17 GB which should be 44.21 GB.

Also, there are different converters/repos. Is it worth creating a shared one?

@Cafe137
Copy link
Collaborator

Cafe137 commented Feb 8, 2024

@LevilkTheReal For depths <= 21, I would maybe remove the message:

Estimated effective volume: 0 B

It looks like a bug.

Instead I would print a human friendly message, something like "effective volume is likely to be smaller than estimated capacity".

What do you think?

@LevilkTheReal
Copy link
Author

LevilkTheReal commented Feb 12, 2024

@LevilkTheReal For depths <= 21, I would maybe remove the message:

Estimated effective volume: 0 B

It looks like a bug.

Instead I would print a human friendly message, something like "effective volume is likely to be smaller than estimated capacity".

What do you think?

Sounds good! What do you think about the fix? Should I just write that message or can I keep the key for the message?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants