Skip to content

Use short array syntax throughout the project#2767

Merged
westonruter merged 4 commits intodevelopfrom
update/short-array-syntax
Jul 8, 2019
Merged

Use short array syntax throughout the project#2767
westonruter merged 4 commits intodevelopfrom
update/short-array-syntax

Conversation

@swissspidy
Copy link
Copy Markdown
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
Copy Markdown
Member

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

@swissspidy
Copy link
Copy Markdown
Collaborator Author

Of course, good call!

@swissspidy swissspidy requested a review from westonruter July 8, 2019 20:44
@swissspidy
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

/me shakes fist at Travis…

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

@westonruter westonruter force-pushed the update/short-array-syntax branch from 90fbb6e to f5c9bf1 Compare July 8, 2019 23:00
@westonruter westonruter force-pushed the update/short-array-syntax branch from f5c9bf1 to ce535e5 Compare July 8, 2019 23:19
@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