-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[3.0] [] for array syntax #12456
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
Comments
I would refrain from making such changing in the core, at least for now. We have much more important things to do and it would (potentially) make merging 2.x branches into 3.x ones more complex. |
Ok, thanks ;) |
fabpot +1, there is no real interest in this. |
@tseho I know a lot of developers interested in changes like this. It's a minor change from a technical point of view, but it delivers a strong message about using modern PHP versions. And you are right about not being spread in PHP projects. That's because people were afraid of moving beyond 5.3. Now that Drupal 8 (5.4), Symfony 3 (5.5) and other big projects are finally upgrading, we could see a quick upgrade movement soon. |
When working on 3.0 changes (BC breaks), it should be very well possible to do so already if you know your code won't be merged back. |
I think it doesn't hurt to use the new syntax for things that are introduced in 3.0. Though I wouldn't change existing code. It would make merging from |
I'm -1 for this syntax. It's less clear, less verbose and doesn't have any advantages |
+1, but it is not a right time. |
👍 for this syntax |
+1 for new code. There is no advantage to change existing code and on the other hand it "breaks" the codes history for no reason. @tseho That it is not widely used in PHP projects is only partially true. It's not widely used in public projects, because of course they want to keep BC. All closed source code I worked on recently used this syntax for new code. It's something different if you know the platform it has to work with. |
Most open source projects I see that have php 5.4 as a min version do actually use the short array syntax. |
-1 for a mix. Inconsistency is the worst thing in a code base imo Op di 11 nov. 2014 22:42 schreef Sebastian Krebs notifications@github.com:
|
@wouterj. I agree there. All or nothing. We have an opportunity for a clean split in 3.0. No point in just making new code use the short array syntax. |
I'm for if (hello() || (true && doSomething([[1, 2, 3], [4, 5, 6]])));
if (hello() || (true && doSomething(array(array(1, 2, 3), array(4, 5, 6))))); The amount of parenthesis will become hard to follow. $a = array_merge([$this->something()], $input->something([$this->getSomeKey()]));
$a = array_merge(array($this->something()), $input->something(array($this->getSomeKey()))); Far more readable. Just don't put it in code that will be merged back to 2.3, 2.5, 2.6 or 3.0 |
@kingcrunch I'm not against using this syntax in closed source projects or in new libraries for PHP 5.5+, I'm just thinking it's a not a good idea to use it in Symfony3, at least for now. If the community start using it only for Symfony3 specific code, we are loosing consistency inside the project. |
+1 for the short array syntax in Symfony 3 |
@iltar I don't think there will be some code that will be merged back to 2.7, there already is a dev branch for 2.7. |
👍 for new arrays [] syntax |
@yguedidi it is the other way actually. 2.7 will receive bug fixes for a long time after the 3.0 release, given it will be a LTS. This means that the 2.7 will be merged into the 3.0 branch on a regular basis (just like done currently for the 2.3 branch) |
👎 for switching now. Because mixing new and old syntaxe in the same project is a bad idea, and because updating the >20 000 ocurances of |
I think it's a great idea to do this (readability++) but only when all supported Symfony versions are pho 5.4+. I guess that's when 3.3 comes out and supersedes 2.7? |
👍 for using the |
+1 for new components, -1 for updating all instances of array(). And from a practical POV, mixed syntax is a necessity, otherwise there will be changes to code simply for syntax purposes, and that seems a waste of resource to me. Maybe 3.3+ changing existing code can be done when changes are implemented in various components, and then when some percentage is done we can migrate the test. It seems to me there will be more negatives by changing code for components that aren't updated often, until a majority of code uses the new syntax. I'm sure devs using the new syntax are also aware of/use/are comfortable with a mix of old & new. |
From my work experience mixing both notations (even in a single file) is less an issue, than some wants to tell here. We are not talking about completely different constructs, but just a different, but similar notation $foo = array('a','b', 'c');
$bar = [1,2,3];
doSomething($foo, $bar); I'd still consider using the new notation for new code and keep the old one for untouched code. One day -- with 3.3 for example -- one can change the remaining occurences, or something like that. Or just keep it as it is 😉 |
👎 replacing And this concerns only internal code, this has no impact on the developer interface of the framework. The only place where it could make sense to use the short array syntax is in the documentation; where the code samples are made to be read. |
@samsonasik I think the question is a bit vague. @javiereguiluz I think the questions asked should be:
In my opinion, replacing is not a good idea. This will make it very hard to merge back into 2.3, 2.6 regarding bug fixes and merging it into 3.0 will cause new When it comes to pull requests to 3.0 and 3.0 only (thus no merging back to 2.*, 3.0 from 2.7), I think it's a good idea to already include the short hand notation. My reasons are simple; It makes code a lot more readable and shorter to write. |
+1 for new code |
I'm 👍 for changing |
@iltar As it was said multiple times, nothing will be merged from the master (3.x) branch into the 2.x branches... the reverse is not true though. If a bug fix is also concerning the 2.x serie, then it should first be based on the 2.x branches and only then merged on 3.x (mainly for compatibility reason) As it is the case right now (i.e you do not backport something from 2.6 into 2.5) |
@Taluu where did you read that I said stuff should be merged from 3.0 to 2.7 and lower? That was not my intention. There are changes that can only be done for 3.0. Those are the changes that I think can get the short notation instead. |
I may have misinterpreted. If that is the case, I'm sorry. :) If you meant the nerge from the 2.x branches (such as the conflicts mentionned by @yguedidi), that is one of the reason why I'm against this change (even though I'm using it everywhere else in my projects...) |
@Taluu I was pointing at merges from 2.7 back into the lower branches, I should not have stated 2.7 with 2.3, etc. I'll change that to avoid more confusion. Edit: I've updated my reply above, I hope it's more clear now :) |
That's why I propose to make the syntax change only once 2.7 is released :) |
👍 for changing Readability as @iltar shows in #12456 (comment) sounds like a win in comparison with the 'polluted' commit history to me. |
Personally, i find the short syntax better when the array is inlined, but as soon as you put the array on multiple lines, i prefer the array() notation... |
Talk about inconsistencies ;) |
Closing as that won't happen. We are using |
Based on http://symfony.com/blog/symfony-3-0-the-roadmap , since Symfony 3.0 will use PHP 5.5 as minimum requirement. Will then the array() syntax be changed to [] ?
There is good tools for that : https://github.com/thomasbachem/php-short-array-syntax-converter
The text was updated successfully, but these errors were encountered: