Skip to content

Made use of "kB" vs. "KiB" consistent #10951

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

Merged
merged 2 commits into from
May 22, 2014
Merged

Conversation

webmozart
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Continuation of #10661.

This PR makes the usage of "kB" and "KiB" consistent across the project. The notations equal the internationally recognized ones:

Short form Long form Value in Bytes
B bytes 1
kB kilobytes 1000
KiB kibibytes 1024
MB megabytes 1000000
MiB mebibytes 1048576
GB gigabytes 1000000000
GiB gibibytes 1073741824

The reason for differentiating between the two is that several users got confused with the current mix (see #10648, #10917, #10661).

FileValidator, UploadedFile and the ProgressBar helper were changed accordingly.

Follow-up feature request: #10962

@Tobion
Copy link
Contributor

Tobion commented May 20, 2014

-1 for the same reason as in #10661 (comment)

@webmozart
Copy link
Contributor Author

@Tobion I read and understand that reason. I also initially implemented the File constraint to handle both SI and 2-based measures when working on this PR.

However, since the PHP manual explicitly states that it interprets "kilobytes" and "megabytes" as being 2-based, I think the best choice is to stick with that convention. I therefore removed the SI-handling code again.

@@ -367,17 +367,17 @@ public function testAnsiColorsAndEmojis()
$this->generateOutput(
" \033[44;37m Starting the demo... fingers crossed \033[0m\n".
" 0/15 ".$progress.str_repeat($empty, 26)." 0%\n".
"
Copy link
Member

Choose a reason for hiding this comment

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

why are you changing this test ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"kB" → "KB"

I also changed the Unicode symbol to the explicit hex notation, since PHPStorm broke the symbol when I saved the file for some reason.

@Tobion
Copy link
Contributor

Tobion commented May 21, 2014

@webmozart But we don't need to continue the mistakes of PHP.

@snoob
Copy link
Contributor

snoob commented May 21, 2014

I agree with @webmozart

@webmozart
Copy link
Contributor Author

@Tobion The alternative, which I had implemented, would be to explicitly differentiate between "kB"/"MB"/"GB"/... and "KiB"/"MiB"/"GiB"/... Since the console component displays memory in the 2-based measure (which is the convention), the console output would therefore also change from e.g. 97 kB to 97 KiB. This output is targeted at end users. The question is: Do end users know what "KiB" means?

I figured that the answer was probably no and removed the code again for that reason, staying in line with PHP's conventions (which were probably picked for the same reason). I don't think there's a benefit in being technically 100% correct here.

@Tobion
Copy link
Contributor

Tobion commented May 21, 2014

The memory output is for developers or other technicians. So they either know it or can look it up in 1 second. I'd prefer not to add to the confusion between binary and SI prefixes and once for all make it clear. Otherwise this debate will continue forever.
But if no other core member agrees I can live with it because I know that we mean binary measures (but all other other users will stay confused).

@Tobion
Copy link
Contributor

Tobion commented May 21, 2014

Btw the memory standards even refer to binary prefixes as alternative. http://en.wikipedia.org/wiki/JEDEC_memory_standards#Unit_prefixes_for_semiconductor_storage_capacity

@webmozart
Copy link
Contributor Author

Ok. I'd like to hear some more opinions though before changing this back.

@jderusse
Copy link
Member

May be a conflict with this PR #10917 ?

@webmozart webmozart changed the title Use "KB" everywhere instead of "kB" Made use of "kB" vs. "KiB" consistent May 22, 2014
@webmozart
Copy link
Contributor Author

Alright, after reviewing #10648, #10917 and #10661, I changed the code and PR description again. We now explicitly differentiate between "kB" and "KiB" to avoid further confusions.

In a follow-up PR for the master branch (2.6), the specification of the "maxSize" option in the File constraint in kibibytes etc. should be supported: #10962

@webmozart
Copy link
Contributor Author

@fabpot You may want to merge this into 2.3, there are no BC concerns involved except for the text output.

@Tobion
Copy link
Contributor

Tobion commented May 22, 2014

+1 now :)

@fabpot fabpot merged commit e4c6da5 into symfony:master May 22, 2014
fabpot added a commit that referenced this pull request May 22, 2014
This PR was merged into the 2.5-dev branch.

Discussion
----------

Made use of "kB" vs. "KiB" consistent

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Continuation of #10661.

This PR makes the usage of "kB" and "KiB" consistent across the project. The notations equal the internationally recognized ones:

Short form | Long form | Value in Bytes
--- | --- | ---
B | bytes | 1
kB | kilobytes | 1000
KiB | kibibytes | 1024
MB | megabytes | 1000000
MiB | mebibytes | 1048576
GB | gigabytes | 1000000000
GiB | gibibytes | 1073741824

The reason for differentiating between the two is that several users got confused with the current mix (see #10648, #10917, #10661).

FileValidator, UploadedFile and the ProgressBar helper were changed accordingly.

Follow-up feature request: #10962

Commits
-------

e4c6da5 [Validator] Improved to-string conversion of the file size/size limit
bbe1045 [Validator][Console][HttpFoundation] Use "KiB" everywhere (instead of "kB")
@webmozart webmozart deleted the issue10661 branch August 4, 2014 14:40
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.

6 participants