Page MenuHomePhabricator

[services] Tests - Add byte size crate
ClosedPublic

Authored by karol on Jun 7 2022, 6:50 AM.
Tags
None
Referenced Files
F3506420: D4227.diff
Fri, Dec 20, 4:33 PM
F3505189: D4227.id13827.diff
Fri, Dec 20, 12:34 PM
F3505188: D4227.id13462.diff
Fri, Dec 20, 12:34 PM
F3505187: D4227.id13397.diff
Fri, Dec 20, 12:34 PM
F3505186: D4227.id13382.diff
Fri, Dec 20, 12:34 PM
F3505172: D4227.id.diff
Fri, Dec 20, 12:34 PM
F3505158: D4227.diff
Fri, Dec 20, 12:31 PM
F3500544: D4227.id13827.diff
Fri, Dec 20, 1:56 AM

Details

Summary

Depends on D4166

Adding bytesize crate

Test Plan

None, this is not used yet, the project can still be built.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

karol edited the test plan for this revision. (Show Details)
karol added reviewers: tomek, varun, max.

do we really need this? i ask because the crate only has 67 stars and hasn't been updated in a while. i think it might be better and safer to just keep the sizes as consts. e.g. const MB: u64 = 1_000_000;. then if you need, say, 4MB you can just do 4 * MB

varun requested changes to this revision.Jun 7 2022, 8:32 AM
This revision now requires changes to proceed.Jun 7 2022, 8:32 AM

i ask because the crate only has 67 stars

fwiw the author/maintainer looks legit: https://www.linkedin.com/in/hyunsikchoi/

Please include me on all diffs that change dependencies! Details on when to add me can be found here

I think 67 stars is fine, it's been updated in the last year, and I agree the maintainer looks legit. That said, I'm concerned that the maintainer doesn't seem to have been answering any GitHub issues recently, and it seems like there are some bugs: https://github.com/hyunsik/bytesize/issues

Ok, so at first I wanted to create something like constants.rs with all the stuff like mb, kb, etc. But then I thought: why would we do it manually if we probably can use some lib that has it all plus some helper functions in case we need any.

I think it is beneficial to use a library. With that said, if you find the lib I picked too poorly maintained or not suitable for whatever reason, we can look for a different one.

As for this lib, I'm not sure if maintenance is that crucial - what can go wrong in defining a few constants? The sizes of kb, mb, gb, tb will never change, right?

Back to your queue as we seem divided.

Conclusion: I'd stay with what's here, I'd definitely avoid doing it manually if possible (and it is).

I don't feel super strongly, personally

This library seems useful. Also, even though it has only a couple of stars, it was downloaded from crates 4M times (for the comparison, the most popular crate (rand) has 128M downloads), so it's used in a lot of projects.

This revision is now accepted and ready to land.Jun 8 2022, 6:23 AM
This revision was automatically updated to reflect the committed changes.