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(build): Use kB in build reporter #10982

Merged
merged 1 commit into from
Nov 22, 2022
Merged

feat(build): Use kB in build reporter #10982

merged 1 commit into from
Nov 22, 2022

Conversation

ArnaudBarre
Copy link
Member

Related to #10895

cc @Shinigami92

@bluwy
Copy link
Member

bluwy commented Nov 18, 2022

Wouldn't dividing by 1024 and showing as KiB be more accurate? Dividing by 1000 feels like it decreases the accuracy

@patak-dev
Copy link
Member

I think we should move to kB (dividing by 1000) too.

The last PR that changed the unit was:

But Chrome dev tools are using kB across the board: https://developer.chrome.com/blog/new-in-devtools-88/#consistent-kb

DevTools now consistently use kB for displaying file/memory sizes. Previously DevTools mixed kB (1000 bytes) and KiB (1024 bytes). For example, the Network panel previously used "kB" labels but actually performed calculations using KiB, which caused needless confusion.

@benmccann
Copy link
Collaborator

For kB, it seems that Ubuntu uses base 10 in all GUI applications (newer) and base 2 in all CLI applications like ls (older).

I feel like it sucks that Chrome and ls don't agree, so there won't be agreement regardless of what we pick.

But if I guess if base 10 is the trend these days we might as well go with it 🤷

@patak-dev
Copy link
Member

That is a good point with ls @benmccann. I still think that it would probably be more surprising that the user doesn't see the same numbers in the chrome network tab compared to what Vite reports, no? Hopefully, every tool ends up moving at one point.

@benmccann
Copy link
Collaborator

Yeah, I'm still slightly in favor of this PR and hope one day ls will end up moving too

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.

4 participants