-
Notifications
You must be signed in to change notification settings - Fork 519
Create embeddable entity for storing common file information #690
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
Conversation
@garak ready |
Entity/File.php
Outdated
*/ | ||
protected $size; | ||
|
||
public function getName() |
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.
What about some phpdoc here? And on other methods, too
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.
I can add phpdocs with param/return types, but I haven't see any profit from method descriptions like Sets the original file name
. It clear from method name what it do :)
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.
So, I push it further: what about type-hinting setters? With default null values, of course
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.
Anyway, I wasn't suggesting to add descriptions, just @param
and @return
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.
@garak done
Entity/File.php
Outdated
*/ | ||
protected $size; | ||
|
||
public function getName() |
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.
So, I push it further: what about type-hinting setters? With default null values, of course
Entity/File.php
Outdated
*/ | ||
protected $size; | ||
|
||
public function getName() |
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.
Anyway, I wasn't suggesting to add descriptions, just @param
and @return
@garak yes, I'l do it later when other codebase will be moved to scalar type hints. Currently we are using this bundle on php 5.5.9 🤣 |
Another one small step to reduce boilerplate when using the bundle.
TODO: