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

many: use common.{Ki,Me,Gi}Bi instead of manually doing them #290

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Nov 29, 2023

This commit uses common.{Ki,Me,Gi}Bi in places that used to write 10 * 1024 * 1024 etc. It's not really important but feels quicker to scan when having the constants.

Also I wonder if we should also have common.{KiB,MiB,GiB,TiB} as a shorthand?

Also just close the PR if you feel this is not worth it :)

ondrejbudai
ondrejbudai previously approved these changes Nov 29, 2023
Copy link
Member

@ondrejbudai ondrejbudai left a comment

Choose a reason for hiding this comment

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

Much better, thanks! :)

cmd/osbuild-playground/playground.go Outdated Show resolved Hide resolved
@bcl
Copy link
Contributor

bcl commented Nov 30, 2023

Also I wonder if we should also have common.{KiB,MiB,GiB,TiB} as a shorthand?

YES :) I don't know anyone who likes to use the long form...

@achilleas-k
Copy link
Member

Also I wonder if we should also have common.{KiB,MiB,GiB,TiB} as a shorthand?

Yes please!

thozza
thozza previously approved these changes Dec 1, 2023
Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@achilleas-k
Copy link
Member

Needs a rebase and merge conflict fixing

This commit uses `common.{Ki,Mi,Gi}B` in places that used to
write `10 * 1024 * 1024` etc. It's not really important but
feels quicker to scan when having the constants.
@mvo5
Copy link
Contributor Author

mvo5 commented Dec 6, 2023

Needs a rebase and merge conflict fixing

Thank you! This is rebased and updated to add/use the shorthand form - fwiw, I like the shorthand a lot better as well :)

Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

LGTM

@thozza thozza added this pull request to the merge queue Dec 6, 2023
Merged via the queue into osbuild:main with commit 82de94a Dec 6, 2023
9 checks passed
@mvo5 mvo5 deleted the use-common-mibi-gibi branch December 6, 2023 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants