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

Switch to binary prefixes #7218

Merged
merged 10 commits into from
Oct 3, 2023
Merged

Switch to binary prefixes #7218

merged 10 commits into from
Oct 3, 2023

Conversation

kanarus
Copy link
Contributor

@kanarus kanarus commented Oct 2, 2023

This is from

and changes the unit of package size to use binary prefixes as shown in test cases in tests/unit/helpers/pretty-bytes-test.js.

@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-frontend 🐹 labels Oct 2, 2023
Copy link
Member

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

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

From https://percy.io/crates-io/crates.io/builds/30429495/changed/1682351587?browser=firefox&browser_ids=49&subcategories=changes_requested,unreviewed&viewLayout=overlay&viewMode=new&width=1280&widths=375,1280

We display KIB in the version list and KiB in the crate view. I suggestion to unify them to KiB. The KIB caused by this CSS:

text-transform: uppercase;

Thanks!

* See https://github.com/rust-lang/crates.io/discussions/7177
*
* Here we set fraction digits to 1 because `cargo publish`
* uses this format (see https://github.com/rust-lang/cargo/blob/master/src/cargo/ops/cargo_package.rs#L168-L171)
Copy link
Member

Choose a reason for hiding this comment

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

Could you please use a tag instead of the master branch here?
Using a fixed version can make sure we can always find it.

Copy link
Contributor Author

@kanarus kanarus Oct 3, 2023

Choose a reason for hiding this comment

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

Thanks, I see! I'll fix them...

app/helpers/pretty-bytes.js Outdated Show resolved Hide resolved
app/helpers/pretty-bytes.js Outdated Show resolved Hide resolved
@Turbo87 Turbo87 merged commit 7e95789 into rust-lang:main Oct 3, 2023
7 checks passed
@Turbo87
Copy link
Member

Turbo87 commented Oct 3, 2023

thanks :)

Comment on lines +9 to +12
prettyBytes(bytes, {
binary: true,
...options,
}),

Choose a reason for hiding this comment

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

@Turbo87, what's the point of "making it possible to override these defaults", if it is never (AFAIU) overridden anywhere?

In #7177 (comment) I intentionally wrote {...options, binary: true} so that binary prefixes will be anywhere and everywhere (that was the point from the start).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend 🐹 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.

4 participants