Skip to content

use ByteSize for byte size rendering / parsing, add same to duration - #3531

Merged
syphar merged 2 commits into
rust-lang:mainfrom
syphar:bytesize
Sep 16, 2026
Merged

syphar merged 2 commits into
rust-lang:mainfrom
syphar:bytesize

Conversation

@syphar

@syphar syphar commented Sep 16, 2026

Copy link
Copy Markdown
Member

This started of as a part of the new docs_rs_build CLI, where I wanted to parse & render byte-sizes & durations consistently.

The lead to doing that for docs_rs_admin too, which will make my life easier when using the admin CLI to work with crate limits.

Then I thought about configuration, where the .env files will also be much easier to read when we use human-readable values for durations & sizes.

Then I fixed all the errors. Also, the Duration type offers some nice constructors since rust 1.91, so I changed that where I saw it. The docs_rs_env_vars change was necessary to handle the Err type from ByteSize / FromStr with our helpers.

These are not yet all places, and not yet the perfect design (I tried to prevent a ByteSize newtype for now).

@syphar syphar self-assigned this Sep 16, 2026
@github-actions github-actions Bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Sep 16, 2026
@syphar
syphar marked this pull request as ready for review September 16, 2026 12:34
@syphar
syphar requested a review from a team as a code owner September 16, 2026 12:34
@syphar

syphar commented Sep 16, 2026

Copy link
Copy Markdown
Member Author

r? @GuillaumeGomez

@rustbot rustbot assigned GuillaumeGomez and unassigned syphar Sep 16, 2026
@GuillaumeGomez

Copy link
Copy Markdown
Member

Is it really worth it to add a new dependency for a type we could implement in less than 100 lines?

@syphar

syphar commented Sep 16, 2026

Copy link
Copy Markdown
Member Author

Is it really worth it to add a new dependency for a type we could implement in less than 100 lines?

I admit I can't judge the compile time difference, it's probably more than these 100 lines with the parsing & rendering, but even when it's 300/400 lines, plus tests?

Comment thread crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs
@GuillaumeGomez

Copy link
Copy Markdown
Member

I don't even think we need 100 lines. We need gb, mb and b constructors. And we can implement Deref (or repr(transparent), this one might be better in this case) on it and basically we have all we need.

@syphar

syphar commented Sep 16, 2026

Copy link
Copy Markdown
Member Author

I don't even think we need 100 lines. We need gb, mb and b constructors. And we can implement Deref (or repr(transparent), this one might be better in this case) on it and basically we have all we need.

I replaced the external crate with a simplified custom implementation, with less unnecessary features or speed, and with two added things that the lib didn't have.

@syphar
syphar merged commit e3ddadd into rust-lang:main Sep 16, 2026
11 checks passed
@syphar
syphar deleted the bytesize branch September 16, 2026 15:20
@github-actions github-actions Bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants