Use new array syntax and make a few minor tweaks #7005
Conversation
bocharsky-bw
force-pushed
the
patch-2
branch
from
7686e13 to
068a35f
Compare
| )); | ||
| return $this->render('lucky/number.html.twig', [ | ||
| 'number' => $number, | ||
| ]); |
There was a problem hiding this comment.
I am against making this change. Using the short array syntax in newer branches makes merging branches up a nightmare.
There was a problem hiding this comment.
@xabbuh Ah, it makes sense. So making changes to the new array syntax is pointless in docs? I'm just wondering for myself for the future.
There was a problem hiding this comment.
@bocharsky-bw yes. I think we should wait till PHP <5.4 is completely dropped in Symfony (that'll be when 2.8 reaches end of maintainance). We can then safely do this change for all code examples in all branches at one go.
| @@ -209,11 +209,11 @@ Bundles are registered in your ``app/AppKernel.php`` file (a rare PHP file in th | |||
| { | |||
| public function registerBundles() | |||
| { | |||
| $bundles = array( | |||
| $bundles = [ | |||
There was a problem hiding this comment.
@xabbuh Could we keep the new array syntax here? We already used it in Symfony SE v3.
There was a problem hiding this comment.
I would say no. If we change the line below this or move the example in some way, we'll always get merge conflicts.
There was a problem hiding this comment.
Thanks for the clarification!
|
I agree with @xabbuh on the short array syntax, but I like the other changes. 👍 If you can update this PR to remove the short array syntax, please do so. Otherwise, we'll do while merging. Status: reviewed |
bocharsky-bw
force-pushed
the
patch-2
branch
from
068a35f to
3217de9
Compare
|
Short array syntax reverted. |
|
Thank you @bocharsky-bw. |
No description provided.