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

Remove PHPCS and replace with ECS #191

Open
wants to merge 51 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
9292206
Add support for amp-onerror on transformed pages (with ESM scripts)
westonruter May 25, 2021
aa6bfe4
Remove PHPCS and replace with ECS
schlessera May 11, 2021
ffaf675
Add ECS config file
schlessera May 11, 2021
610860a
Adapt config
schlessera May 11, 2021
1f8d849
Fix spacing
schlessera May 11, 2021
9b29470
Reorder imports
schlessera May 11, 2021
59b1adf
Add missing trailing commas
schlessera May 11, 2021
028d593
Adapt ECS config
schlessera May 11, 2021
2f8ed35
Fix indentation
schlessera May 11, 2021
17c6c67
Include ECS in workflow
schlessera May 11, 2021
d44f46a
Include ECS config file in checks
schlessera May 11, 2021
e116b98
Rename composer cbf to composer fix
schlessera May 11, 2021
311a2d4
Add documentation code style checks
schlessera May 11, 2021
62b823c
Fix CS in readme file
schlessera May 11, 2021
d89cbb0
Remove ECS on lower PHP versions
schlessera May 11, 2021
4b9d6f5
Ignore platform reqs when removing ECS
schlessera May 11, 2021
36d5ef6
Remove ECS from --dev
schlessera May 11, 2021
7844b42
Stick to platform requirements
schlessera May 11, 2021
6d04c97
Ignore ECS config file for code coverage
schlessera May 26, 2021
fea0f92
Fix ECS issue in new exception
schlessera May 26, 2021
62a13f2
Sync local fallback files - 2021-05-27
schlessera May 27, 2021
9031492
Merge pull request #213 from ampproject/sync-local-fallback-files
github-actions[bot] May 27, 2021
de80973
Merge pull request #211 from ampproject/add/amp-onerror-transformed
westonruter May 27, 2021
c40223b
Sync local fallback files - 2021-05-28
schlessera May 28, 2021
183c8ad
Sync spec test suite - 2021-05-28
schlessera May 28, 2021
b969668
Merge pull request #214 from ampproject/sync-local-fallback-files
github-actions[bot] May 28, 2021
da2aaf0
Merge pull request #215 from ampproject/sync-spec-test-suite
schlessera May 28, 2021
5f774d8
Sync local fallback files - 2021-05-29
schlessera May 29, 2021
f481fae
Merge pull request #216 from ampproject/sync-local-fallback-files
schlessera May 29, 2021
9988d3f
Sync local fallback files - 2021-05-30
schlessera May 30, 2021
178c7ac
Merge pull request #217 from ampproject/sync-local-fallback-files
github-actions[bot] May 30, 2021
6355b26
Remove PHPCS and replace with ECS
schlessera May 11, 2021
73d5ce3
Add ECS config file
schlessera May 11, 2021
c4cccc6
Adapt config
schlessera May 11, 2021
abb2ddd
Fix spacing
schlessera May 11, 2021
6924461
Reorder imports
schlessera May 11, 2021
fdccdb6
Add missing trailing commas
schlessera May 11, 2021
0325be8
Adapt ECS config
schlessera May 11, 2021
e78518f
Fix indentation
schlessera May 11, 2021
24db01a
Include ECS in workflow
schlessera May 11, 2021
ee20172
Include ECS config file in checks
schlessera May 11, 2021
ac2cff8
Rename composer cbf to composer fix
schlessera May 11, 2021
d7c4365
Add documentation code style checks
schlessera May 11, 2021
7a2dc65
Fix CS in readme file
schlessera May 11, 2021
355f0dd
Remove ECS on lower PHP versions
schlessera May 11, 2021
fd5f8b3
Ignore platform reqs when removing ECS
schlessera May 11, 2021
c7ec671
Remove ECS from --dev
schlessera May 11, 2021
66d2710
Stick to platform requirements
schlessera May 11, 2021
4015916
Ignore ECS config file for code coverage
schlessera May 26, 2021
a33c49b
Fix ECS issue in new exception
schlessera May 26, 2021
14cfef6
Fix merge conflict
schlessera Nov 25, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ coverage:

# See https://docs.codecov.io/docs/ignoring-paths
ignore:
- "/ecs.php"
- "/bin"

comment: false
15 changes: 11 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ jobs:
restore-keys: |
${{ runner.os }}-composer-

- name: Remove ECS on lower PHP versions
if: ${{ matrix.php == '5.6' || matrix.php == '7.0' }}
run: composer remove --dev symplify/easy-coding-standard -W

- name: Install Composer dependencies
run: composer install

Expand Down Expand Up @@ -75,7 +79,7 @@ jobs:
strategy:
fail-fast: false
matrix:
php: ['7.4']
php: ['8.0']

steps:
- name: Checkout
Expand All @@ -86,7 +90,7 @@ jobs:
with:
php-version: ${{ matrix.php }}
extensions: dom, iconv, json, libxml, zip
tools: composer, cs2pr
tools: composer

- name: Get Composer Cache Directory
id: composer-cache
Expand All @@ -102,8 +106,11 @@ jobs:
- name: Install Composer dependencies
run: composer install

- name: Detect coding standard violations (PHPCS)
run: vendor/bin/phpcs -q --report=checkstyle --runtime-set ignore_errors_on_exit 1 --runtime-set ignore_warnings_on_exit 1 | cs2pr --graceful-warnings
- name: Detect coding standard violations in source code
run: composer cs

- name: Detect coding standard violations in documentation
run: composer cs-md

static-analysis:
name: Static analysis / PHP ${{ matrix.php }}
Expand Down
168 changes: 84 additions & 84 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use AmpProject\Optimizer\ErrorCollection;
use AmpProject\Optimizer\TransformationEngine;

$transformationEngine = new TransformationEngine(); // 1.
$errorCollection = new ErrorCollection; // 2.
$errorCollection = new ErrorCollection(); // 2.

$optimizedHtml = $transformationEngine->optimizeHtml( // 3.
$unoptimizedHtml, // 4.
Expand Down Expand Up @@ -90,7 +90,7 @@ if (! $dom instanceof Document) {
}

$transformationEngine = new TransformationEngine();
$errorCollection = new ErrorCollection;
$errorCollection = new ErrorCollection();
$transformationEngine->optimizeDom($dom, $errorCollection);
```

Expand All @@ -103,32 +103,32 @@ The [`AmpProject\Optimizer\ErrorCollection`](https://github.com/ampproject/amp-t
To check whether errors were found, you can iterate over the collection, which will provide you with 0 or more [`AmpProject\Optimizer\Error`](https://github.com/ampproject/amp-toolbox-php/blob/main/src/Optimizer/Error.php) objects.

```php
$errorCollection = new ErrorCollection;
$errorCollection = new ErrorCollection();

// Do the transformation here, while passing in the $errorCollection object.

foreach ($errorCollection as $error) {
printf(
"Error code: %s\nError Message: %s\n",
$error->getCode(),
$error->getMessage()
);
printf(
"Error code: %s\nError Message: %s\n",
$error->getCode(),
$error->getMessage()
);
}
```

A quick count of the errors can be done for early returns as needed:

```php
if ($errorCollection->count() > 0) {
$this->log('The AMP serverside optimization process produced one or more errors.');
$this->log('The AMP serverside optimization process produced one or more errors.');
}
```

You can check whether the collection of errors contains an error with a specific code as well. The current convention is that all errors have their class shortname (the class name without the namespace) as the error code.

```php
if ($errorCollection->has('CannotRemoveBoilerplate')) {
$this->log('The boilerplate was not removed by the Optimizer.');
$this->log('The boilerplate was not removed by the Optimizer.');
}
```

Expand Down Expand Up @@ -161,15 +161,15 @@ use AmpProject\Optimizer\TransformationEngine;
use AmpProject\Optimizer\Transformer;

$configurationData = [
Configuration::KEY_TRANSFORMERS => [
Transformer\ServerSideRendering::class,
Transformer\AmpRuntimeCss::class,
Transformer\TransformedIdentifier::class,
],
Configuration::KEY_TRANSFORMERS => [
Transformer\ServerSideRendering::class,
Transformer\AmpRuntimeCss::class,
Transformer\TransformedIdentifier::class,
],
];

$transformationEngine = new TransformationEngine(
new DefaultConfiguration($configurationData)
new DefaultConfiguration($configurationData)
);
```

Expand All @@ -186,13 +186,13 @@ use AmpProject\Optimizer\TransformationEngine;
use AmpProject\Optimizer\Transformer;

$configurationData = [
Transformer\AmpRuntimeCss::class => [
Configuration\AmpRuntimeCssConfiguration::CANARY => true,
],
Transformer\AmpRuntimeCss::class => [
Configuration\AmpRuntimeCssConfiguration::CANARY => true,
],
];

$transformationEngine = new TransformationEngine(
new DefaultConfiguration($configurationData)
new DefaultConfiguration($configurationData)
);
```

Expand Down Expand Up @@ -220,16 +220,16 @@ use AmpProject\Optimizer\TransformationEngine;
use MyProject\MyCustomTransformer;

$configurationData = [
Configuration::KEY_TRANSFORMERS => array_merge(
Configuration::DEFAULT_TRANSFORMERS,
[
MyCustomTransformer::class
],
),
Configuration::KEY_TRANSFORMERS => array_merge(
Configuration::DEFAULT_TRANSFORMERS,
[
MyCustomTransformer::class,
],
),
];

$transformationEngine = new TransformationEngine(
new DefaultConfiguration($configurationData)
new DefaultConfiguration($configurationData)
);
```

Expand All @@ -249,22 +249,22 @@ use MyProject\MyCustomTransformer;
use MyProject\MyCustomTransformerConfiguration;

$configurationData = [
Configuration::KEY_TRANSFORMERS => array_merge(
Configuration::DEFAULT_TRANSFORMERS,
[
MyCustomTransformer::class
],
),
MyCustomTransformer::class => [
MyCustomTransformerConfiguration::SOME_CONFIG_KEY => 'some value',
],
Configuration::KEY_TRANSFORMERS => array_merge(
Configuration::DEFAULT_TRANSFORMERS,
[
MyCustomTransformer::class,
],
),
MyCustomTransformer::class => [
MyCustomTransformerConfiguration::SOME_CONFIG_KEY => 'some value',
],
];

$configuration = new DefaultConfiguration($configurationData);

$configuration->registerConfigurationClass(
MyCustomTransformer::class,
MyCustomTransformerConfiguration::class
MyCustomTransformer::class,
MyCustomTransformerConfiguration::class
);

$transformationEngine = new TransformationEngine($configuration);
Expand All @@ -285,24 +285,24 @@ use AmpProject\Optimizer\Configuration\BaseTransformerConfiguration;

final class MyCustomTransformerConfiguration extends BaseTransformerConfiguration
{
const SOME_CONFIG_KEY = 'some_config_key';

protected function getAllowedKeys()
{
return [
self::SOME_CONFIG_KEY => 'default value',
];
}

protected function validate($key, $value)
{
switch ($key) {
case self::SOME_CONFIG_KEY:
// Validate configuration value here.
}

return $value;
}
const SOME_CONFIG_KEY = 'some_config_key';

protected function getAllowedKeys()
{
return [
self::SOME_CONFIG_KEY => 'default value',
];
}

protected function validate($key, $value)
{
switch ($key) {
case self::SOME_CONFIG_KEY:
// Validate configuration value here.
}

return $value;
}
}
```

Expand Down Expand Up @@ -349,40 +349,40 @@ This layer of abstraction allows code outside of the transformation engine to co
namespace MyProject;

use AmpProject\Dom\Document;
use AmpProject\RemoteGetRequest;
use AmpProject\Optimizer\ErrorCollection;
use AmpProject\Optimizer\Transformer;
use AmpProject\RemoteGetRequest;
use Throwable;

final class MyCustomTransformer implements Transformer
{
const END_POINT = 'https://example.com/some_endpoint/';
const END_POINT = 'https://example.com/some_endpoint/';

private $remoteRequest;
private $remoteRequest;

public function __construct(RemoteGetRequest $remoteRequest)
{
$this->remoteRequest = $remoteRequest;
}
public function __construct(RemoteGetRequest $remoteRequest)
{
$this->remoteRequest = $remoteRequest;
}

public function transform(Document $document, ErrorCollection $errors)
{
try {
$response = $this->remoteRequest->get(self::END_POINT);
} catch (Throwable $exception) {
// Add error handling here.
}
public function transform(Document $document, ErrorCollection $errors)
{
try {
$response = $this->remoteRequest->get(self::END_POINT);
} catch (Throwable $exception) {
// Add error handling here.
}

$statusCode = $response->getStatusCode();
$statusCode = $response->getStatusCode();

if (200 < $statusCode || $statusCode >= 300) {
// Add error handling here.
}
if (200 < $statusCode || $statusCode >= 300) {
// Add error handling here.
}

$content = $response->getBody();
// Make use of the $content you've just retrieved from an external source.
}
$content = $response->getBody();

// Make use of the $content you've just retrieved from an external source.
}
}
```

Expand All @@ -395,10 +395,10 @@ use AmpProject\Optimizer\DefaultConfiguration;
use AmpProject\Optimizer\TransformationEngine;

$transformationEngine = new TransformationEngine(
new DefaultConfiguration(),
new DefaultConfiguration(),

// A custom implementation that lets you control how remote requests are handled.
new MyCustomRemoteGetRequestImplementation()
// A custom implementation that lets you control how remote requests are handled.
new MyCustomRemoteGetRequestImplementation()
);
```

Expand All @@ -423,12 +423,12 @@ use AmpProject\RemoteRequest\FallbackRemoteGetRequest;
use AmpProject\RemoteRequest\FilesystemRemoteGetRequest;

const FALLBACK_MAPPING = [
'https://example.com/some_endpoint/' => __DIR__ . '/../fallback_files/some_endpoint.json',
'https://example.com/some_endpoint/' => __DIR__ . '/../fallback_files/some_endpoint.json',
];

$remoteRequest = new FallbackRemoteGetRequest(
new CurlRemoteGetRequest(true, 5, 0), // 5 second timeout with no retries, and ...
new FilesystemRemoteGetRequest(self::FALLBACK_MAPPING) // ... fall back to shipped files.
new CurlRemoteGetRequest(true, 5, 0), // 5 second timeout with no retries, and ...
new FilesystemRemoteGetRequest(self::FALLBACK_MAPPING) // ... fall back to shipped files.
);

$transformationEngine = new TransformationEngine(new DefaultConfiguration(), $remoteRequest);
Expand Down
18 changes: 9 additions & 9 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,10 @@
"require-dev": {
"ext-zip": "*",
"civicrm/composer-downloads-plugin": "^2.1 || ^3.0",
"dealerdirect/phpcodesniffer-composer-installer": "0.7.1",
"php-parallel-lint/php-parallel-lint": "^1.2",
"phpcompatibility/phpcompatibility-wp": "2.1.1",
"phpunit/phpunit": "^5 || ^6 || ^7 || ^8 || ^9",
"roave/security-advisories": "dev-master",
"sirbrillig/phpcs-variable-analysis": "2.11.0",
"squizlabs/php_codesniffer": "^3",
"symplify/easy-coding-standard": "^9.3",
"yoast/phpunit-polyfills": "^0.2.0"
},
"suggest": {
Expand Down Expand Up @@ -53,17 +50,20 @@
}
},
"scripts": {
"cbf": "phpcbf",
"cs": "if [ -z $TEST_SKIP_PHPCS ]; then phpcs; fi",
"lint": "if [ -z $TEST_SKIP_LINTING ]; then parallel-lint -j 10 --colors --exclude vendor .; fi",
"fix": "ecs check --fix",
"fix-md": "ecs check-markdown README.md --fix",
"cs": "ecs check",
"cs-md": "ecs check-markdown README.md",
"lint": "parallel-lint -j 10 --colors --exclude vendor .",
"test": [
"@lint",
"@unit",
"@cs",
"@cs-md",
"@analyze"
],
"analyze": "if [ -z $TEST_SKIP_PHPSTAN ]; then phpstan --version; phpstan analyze --ansi; fi",
"unit": "if [ -z $TEST_SKIP_PHPUNIT ]; then phpunit --colors=always; fi",
"analyze": "phpstan --version; phpstan analyze --ansi",
"unit": "phpunit --colors=always",
"sync-fallback-files": "bin/sync-amp-runtime-local-fallback-resources.php",
"sync-test-specs": "rm -rf tests/spec && bin/sync-amp-toolbox-test-suite.php",
"sync": [
Expand Down
Loading