-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Added a note about coding standards and method arguments #6618
Conversation
@@ -160,6 +160,9 @@ Structure | |||
``tearDown`` methods of PHPUnit tests, which should always be the first methods | |||
to increase readability; | |||
|
|||
* Declare all the arguments in the same line as the method/function name, no |
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.
Class name 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'm sorry I don't understand this comment.
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.
See the second example of http://www.php-fig.org/psr/psr-2/#4-1-extends-and-implements
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.
Something like this, inspired from @wouterj link.
Right:
class ClassName extends ParentClass implements \ArrayAccess, \Countable
{
// constants, properties, methods
}
Wrong:
class ClassName extends ParentClass implements
\ArrayAccess,
\Countable,
\Serializable
{
// constants, properties, methods
}
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.
Thanks for explaining it to me. In 59a09bf I've proposed something for this.
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.
this has to be "on the same line" as well
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.
Done too. Thanks.
@@ -153,13 +153,19 @@ Structure | |||
that are not intended to be instantiated from the outside and thus are not | |||
concerned by the `PSR-0`_ and `PSR-4`_ autoload standards; | |||
|
|||
* Declare the class inheritance (if any) and all the implemented interfaces (if | |||
any) in the same line as the class name; |
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.
on the same line (same below)
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 don't think "if any" is needed here.
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 agree. Done both changes. Thanks.
LGTM. 👍 |
👍 |
👍 |
Thank you Javier. |
…(javiereguiluz) This PR was submitted for the 2.3 branch but it was merged into the 2.7 branch instead (closes #6618). Discussion ---------- Added a note about coding standards and method arguments This coding standard is optional in the PSR-2 standard, so we must explain our own convention. Related to symfony/symfony#18890. Commits ------- b7429a5 Added a note about coding standards and method arguments
This coding standard is optional in the PSR-2 standard, so we must explain our own convention. Related to symfony/symfony#18890.