fixed cs of php parts #1497
fixed cs of php parts #1497
Conversation
| @@ -137,7 +137,7 @@ Next, we create the data transformer, which does the actual conversion:: | |||
| /** | |||
| * Transforms a string (number) to an object (issue). | |||
| * | |||
| * @param string $number | |||
| * @param string $number | |||
There was a problem hiding this comment.
This is not a standard, you need to align all variable names and there description, but not the variable name and a description of another item without a variable name (e.g. @throws)
There was a problem hiding this comment.
php cs fixer was used
There was a problem hiding this comment.
Yea, this seems odd - what if $number has a description? Then it wouldn't line up with the description of the @throws or the Issue (if it had one).
I can't really argue with the phpcs, but this one almost feels wrong :/
There was a problem hiding this comment.
@weaverryan reverted ;)
|
Most of the fixes are because you want to add a new line between the first comment (with file info) and the code. I disagree on that. It is used over the complete documentation and isn't part of the actual script, it is just a reminder of what file is edited/added, and I hope nobody copy that in it's project. And if you put an empty line in de PHP code, you need to put it also in the Twig code examples and all the other codings (like Yaml and XML), a lot of work on something that isn't necessary. |
| namespace Acme\WebserviceUserBundle\Security\User; | ||
|
|
||
| use Symfony\Component\Security\Core\User\UserInterface; | ||
|
|
||
| class WebserviceUser implements UserInterface |
There was a problem hiding this comment.
This is not a phpcs issue - and this class is referenced in a few other places in this doc. I actually kind of like the verbose name (though maybe for phpcs it should be WebServiceUser) because it's easier to follow along with.
There was a problem hiding this comment.
@weaverryan I changed the class name to make it fit with the filename... (oops I should have done the invert change the filename to fit the class name (according to PSR-0)) now it is fixed
|
Hey Brikou! I really like this PR :). But, I agree with @wouterj that I don't like the added extra line between the filename comment and the namespace. I'm happily willing to be out-voted if enough people care. For me, in the docs, it's an issue of space - I want to help people know what file we're talking about but without creating too much space before what's really important. Thanks! |
|
@weaverryan now I think everything is fine, should be mergable I guess, cheer ;) |
| @@ -209,8 +209,6 @@ inside your ``AcmeHelloBundle``:: | |||
| // src/Acme/HelloBundle/Controller/HelloController.php | |||
| namespace Acme\HelloBundle\Controller; | |||
|
|
|||
| use Symfony\Component\HttpFoundation\Response; | |||
There was a problem hiding this comment.
This must be reverted, because it is used in the next code block (and this code is included there because of the ...).
Or, maybe better, place this line in the next code block. I think that's the best option.
There was a problem hiding this comment.
@wouterj done thank you ;)
|
Hey @brikou This looks really great now :). But can you re-open a new PR that's against the 2.0 branch - this won't patch cleanly to 2.0 and it should apply backwards. Thanks! |
|
Hi @weaverryan... it was a pain but here it is #1534 |
|
@weaverryan ping ;) |
No description provided.