Skip to content
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

EZP-29373: [CS] Switched codebase to use short array syntax #2597

Merged
merged 2 commits into from Jun 23, 2019

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Apr 4, 2019

Question Answer
JIRA issue EZP-29373
Improvement yes
New feature no
Target version 6.7, 6.13, 7.4, 7.5, master
BC breaks no
Tests pass yes
Doc needed no

This PR switches long array syntax to the short one.
Due to maintenance issues we need to do it on all supported branches.

TODO:

  • Switch codebase to short array syntax.
  • Configure php-cs-fixer to enforce short array syntax.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Confirm Travis agrees.
  • Ask for Code Review.

@ezrobot

This comment has been minimized.

@andrerom
Copy link
Contributor

andrerom commented Apr 16, 2019

LIKE! But I then assume we aim to do this as on PR, and then a follower to upgrade CS fixer so we can fully support PHP 7.3 and up?

If so, these smaller adjustment to CS works pretty well:
ezsystems/LegacyBridge@13c75de


Side: I have hidden the comment by ezrobot as we have as beginning last week disabled CS check jobs on ci.ez.no (or at least should have done)
This means we should probably add a job here to check CS + composer validate while at it.

@alongosz
Copy link
Member Author

LIKE! But I then assume we aim to do this as on PR, and then a follower to upgrade CS fixer so we can fully support PHP 7.3 and up?
If so, these smaller adjustment to CS works pretty well:
ezsystems/LegacyBridge@13c75de

I tried to do everything at once, though by different rule-set (I can try the one from LegacyBridge), but after looking at the diff, it seemed too much. I plan to move to fixer v2.14 as a follow-up.

Side: I have hidden the comment by ezrobot as we have as beginning last week disabled CS check jobs on ci.ez.no (or at least should have done)
This means we should probably add a job here to check CS + composer validate while at it.

Good point :)

@gggeek
Copy link
Contributor

gggeek commented Jun 19, 2019

Not really convinced that this is a net benefit to end users :-( Rationale explained in the Jira ticket.

@alongosz
Copy link
Member Author

@gggeek foremost this is to benefit eZ Engineering and Support Teams as an internal change. We're the ones maintaining it and working the most on the codebase. We've been postponing this for over 2 years and now it's time to do it. Symfony recently did the same thing and it was also done for 3.x and 4.x, which clearly shows that they see maintenance issues here as well.

Maintenance outweighs necessity to go through one more commit when checking diff.
Answered also in JIRA ticket.

@alongosz alongosz merged commit 0f2ea82 into ezsystems:6.7 Jun 23, 2019
@alongosz alongosz deleted the ezp-29373-short-array-syntax branch June 23, 2019 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants