Skip to content

og_image: Use binary prefixes for format_bytes #11531

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

Merged
merged 2 commits into from
Jul 7, 2025

Conversation

T0mstone
Copy link
Contributor

@T0mstone T0mstone commented Jul 6, 2025

Usage of SI prefixes to denote powers of 1024 is incorrect:

[T]he major standards organizations have expressly disapproved the use of SI prefixes to denote binary multiples, and recommended or mandated the use of the IEC prefixes for that purpose[.]

This just changes them to use the proper kiB and MiB instead of kB and MB.

An alternative would be changing the code instead, to use powers of 1000 like format_number does, but that might be a bit ambiguous precisely because this error is still so common.

@T0mstone
Copy link
Contributor Author

T0mstone commented Jul 6, 2025

Oh, looking at the Wiki page again, I guess it should be Ki instead of ki.

@T0mstone
Copy link
Contributor Author

T0mstone commented Jul 6, 2025

I don't understand why the tests are failing in CI but passing on my machine.

@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-backend ⚙️ labels Jul 7, 2025
T0mstone and others added 2 commits July 7, 2025 07:08
Technically `k` is correct, but for casing consistency with `KiB` we'll use uppercase now...
@Turbo87 Turbo87 force-pushed the binary-prefixes branch from 9c913af to 6a41d5d Compare July 7, 2025 05:08
@Turbo87
Copy link
Member

Turbo87 commented Jul 7, 2025

I don't understand why the tests are failing in CI but passing on my machine.

Might have been related to #11530

I've rebased it and CI seems to be happy now

@Turbo87 Turbo87 merged commit e9a0c0b into rust-lang:main Jul 7, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants