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

Convert::memstring2bytes should return integer value #8583

Merged
merged 1 commit into from
Nov 7, 2018

Conversation

wernerkrauss
Copy link
Contributor

bytes are by nature an integer

fixes #8572

Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

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

This is potentially a semver violation. If people were to introspect the return type of this method they'd see it change from float to int, but seeing as we always round the float return value of round(), it'd be an int anyway. LGTM 👍

@unclecheese
Copy link

FYI This broke the silverstripe/assets test, which is expecting a float. https://github.com/silverstripe/silverstripe-assets/blob/1/tests/php/FileTest.php#L701

@robbieaverill
Copy link
Contributor

Ha! Damn. Do you think we should revert @unclecheese, or update that test?

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.

None yet

4 participants