Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpFoundation] Incorrect documentation and method name for UploadedFile::getClientSize()#25324
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
50794b6 todf51428CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
reasonable 👍
where's the picture/photo? 🤔
| */ | ||
| publicfunctiongetClientSize() | ||
| { | ||
| @trigger_error(sprintf('%s is deprecated since 4.1 and will be removed in 5.0 use %s::%s.',__METHOD__,__CLASS__,'getSize'),E_USER_DEPRECATED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"%s" ... 5.0. Use getFileSize() instead.
| * | ||
| * @return int|null The file size | ||
| */ | ||
| publicfunctiongetFileSize() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
getClientSize becomesgetSize?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
which you are actually suggesting above :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's not the same thing I guess,getSize already exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Right, my bad.\SplFileInfo::getSize — Gets file size.
So what dowe suggest here? Another file size? Or should we override getSize to avoid re-calculating / forward if $size is null.
getUploadedSize maybe? Would it make sense?
df51428 toeb469dcCompareSimperfit commentedDec 5, 2017
@ro0NL There are no picture on this one because i've pushed 2 PR this morning and didn't take the time to take one picture for this one ^^'. |
| */ | ||
| publicfunctiongetClientSize() | ||
| { | ||
| @trigger_error(sprintf('%s is deprecated since 4.1 and will be removed in 5.0 use %s instead.',__METHOD__,'getFileSize'),E_USER_DEPRECATED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We really want the first%s to be quoted i guess for HTML purpose (e.g. make it bold in the profiler log panel).
Also i suggested to start a new sentence as of5.0. Use ... instead (no real reason to use a sprintf placeholder here also).
Sorry :) just following 3.x convention AFAIK.
157f01a tof5a8168Compare| $this->mimeType =$mimeType ?:'application/octet-stream'; | ||
| $this->size =$size; | ||
| if (null !==$size) { | ||
| @trigger_error('Passing a size in the constructor is deprecated since 4.1 and will be removed in 5.0. Use getSize instead.',E_USER_DEPRECATED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"getSize()"
| */ | ||
| publicfunctiongetClientSize() | ||
| { | ||
| @trigger_error(sprintf('"%s" is deprecated since 4.1 and will be removed in 5.0. Use getSize instead.',__METHOD__),E_USER_DEPRECATED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
same here
f5a8168 toa8e4d7aCompare…File::getClientSize()
a8e4d7a tob136320CompareSimperfit commentedDec 11, 2017
The PR has been fixed from review and rebased with current master. |
fabpot commentedDec 11, 2017
Thank you@Simperfit. |
…me for UploadedFile::getClientSize() (Simperfit)This PR was merged into the 4.1-dev branch.Discussion----------[HttpFoundation] Incorrect documentation and method name for UploadedFile::getClientSize()| Q | A| ------------- | ---| Branch? | 4.1| Bug fix? | no| New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks? | no| Deprecations? | yes <!-- don't forget to update UPGRADE-*.md files -->| Tests pass? | yes| Fixed tickets |#23791| License | MIT| Doc PR | noneReplace the wrong named and documented method by another with better doc and better namingCommits-------b136320 [HttpFoundation] Incorrect documentation and method name for UploadedFile::getClientSize()
Uh oh!
There was an error while loading.Please reload this page.
Replace the wrong named and documented method by another with better doc and better naming