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

Use short array syntax throughout the project #2767

Merged
merged 4 commits into from
Jul 8, 2019

Conversation

swissspidy
Copy link
Collaborator

This is a bit of a follow-up to #2493.

The main plugin file, amp.php, is left untouched as it still should be parseable on PHP 5.2.

Also does not change includes/sanitizers/class-amp-allowed-tags-generated.php as it is auto-generated.

This is a bit of a follow-up to #2493.

The main plugin file, `amp.php`, is left untouched as it still should be parseable on PHP 5.2.

Also does not change `includes/sanitizers/class-amp-allowed-tags-generated.php` as it is auto-generated.
@googlebot googlebot added the cla: yes Signed the Google CLA label Jul 8, 2019
@swissspidy swissspidy added P2 Low priority Task Tasks which do not involve engineering labels Jul 8, 2019
@swissspidy swissspidy marked this pull request as ready for review July 8, 2019 19:33
@westonruter
Copy link
Member

As part of this, the PHPCS ruleset should be updated for force the preferred syntax for consistency.

@swissspidy
Copy link
Collaborator Author

Of course, good call!

@swissspidy
Copy link
Collaborator Author

The file is currently excluded from phpcs, and I left it out on purpose because it's auto-generated.

Fun Fact: PHPCS actually crashes locally for me because the file is so huge.

I can update the generator to output the bracketed syntax as well.

Sounds good to me!

@westonruter
Copy link
Member

Fun Fact: PHPCS actually crashes locally for me because the file is so huge.

As part the plugin v2.0 effort, I'd like to revisit that file entirely, look at ways it can be made smaller and/or broken up into smaller files.

@westonruter westonruter added this to the v1.2.1 milestone Jul 8, 2019
@swissspidy
Copy link
Collaborator Author

I was curious:

$ php -d memory_limit=4096M vendor/bin/phpcbf includes/sanitizers/class-amp-allowed-tags-generated.php

PHPCBF RESULT SUMMARY
-----------------------------------------------------------------------------
FILE                                                         FIXED  REMAINING
-----------------------------------------------------------------------------
includes/sanitizers/class-amp-allowed-tags-generated.php     15247  9
-----------------------------------------------------------------------------
A TOTAL OF 15247 ERRORS WERE FIXED IN 1 FILE
-----------------------------------------------------------------------------

Time: 1 mins, 60 secs; Memory: 12MB

😅😅😅

@westonruter
Copy link
Member

/me shakes fist at Travis…

git diff develop...6bfa9746c7fb6c5c09a56667ff5c1c919c984901
fatal: Path 'amp.php' exists on disk, but not in '6bfa9746c7fb6c5

@westonruter westonruter merged commit 7feaf96 into develop Jul 8, 2019
@westonruter westonruter deleted the update/short-array-syntax branch July 8, 2019 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Signed the Google CLA P2 Low priority Task Tasks which do not involve engineering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants