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

Use /internal/memory constants #1354

Merged
merged 2 commits into from
Apr 18, 2022
Merged

Conversation

dcantah
Copy link
Contributor

@dcantah dcantah commented Apr 14, 2022

We have a bunch of 1024 * 1024 or 1024 * 1024 * 1024 numerical constants (or just other named megabyte constants) lying around. This changes to using the constants we have defined in the /internal/memory package instead.

This change additionally renames the memory.MegaByte and memory.GigaByte constants to Mib and GiB respectively, and leaves them as untyped consts.

We have a bunch of 1024 * 1024 or 1024 * 1024 * 1024 numerical constants
(or just other megabyte constants) lying around. This just changes to using
the constants we have defined in the /internal/memory package.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
Copy link
Contributor

@helsaawy helsaawy left a comment

Choose a reason for hiding this comment

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

ive been wanting this for a while 😅

internal/memory/types.go Outdated Show resolved Hide resolved
MegaByte = uint64(1024 * 1024)
GigaByte = 1024 * MegaByte
MegaByte uint64 = 1024 * 1024
GigaByte uint64 = 1024 * MegaByte
Copy link
Member

Choose a reason for hiding this comment

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

Technically this should be Gibibyte. Maybe we can fix that too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, maybe we just shorten these to MiB and GiB then as well. Thoughts? cc @anmaxvl

Copy link
Member

Choose a reason for hiding this comment

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

I like that idea even more :)

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good to me 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changin in a follow up commit that we can squash at the end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

* Change /internal/memory constants to MiB and GiB from MegaByte and
GigaByte.
* Leave constants untyped.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah dcantah merged commit a4c9777 into microsoft:master Apr 18, 2022
anmaxvl added a commit that referenced this pull request Feb 7, 2023
Sync ADO with upstream to enable including test GCS binaries as
part of dev-pipeline

Related work items: #1311, #1322, #1341, #1343, #1345, #1347, #1348, #1350, #1353, #1354, #1355, #1358, #1361, #1365, #1368, #1369, #1370
princepereira pushed a commit to princepereira/hcsshim that referenced this pull request Aug 29, 2024
* Use /internal/memory constants

We have a bunch of 1024 * 1024 or 1024 * 1024 * 1024 numerical constants
(or just other megabyte constants) lying around. This changes to using
the constants we have defined in the /internal/memory package.

This additionally changes the names of the constants from MegaByte/GigaByte to MiB/GiB and
changes them to untyped constants.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
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