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

Raise PHPStan level to 4 in lib/optimizer #4685

Merged
merged 7 commits into from
May 15, 2020

Conversation

schlessera
Copy link
Collaborator

@schlessera schlessera commented May 12, 2020

Summary

This PR fixes the following issues that PHPStan detected at level 4:

Fixes #4380

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@schlessera schlessera added Bug Something isn't working Optimizer labels May 12, 2020
@googlebot googlebot added the cla: yes Signed the Google CLA label May 12, 2020
@schlessera schlessera linked an issue May 12, 2020 that may be closed by this pull request
@schlessera schlessera changed the title Raise PHPStan level to 4 Raise PHPStan level to 4 in lib/optimizer May 12, 2020
if (200 < $statusCode || $statusCode >= 300) {
if ($statusCode < 200 || $statusCode >= 300) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes again, a good candidate for backporting.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, the changes are all in separate commits, so backporting the above should be a straight-forward cherry-pick.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The branch doesn't merge cleanly into 1.5, and 83018f2 doesn't merge cleanly either. But I'll apply this one fix manually, and leave the other changes to 1.6 alone.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See f9f10d1.

lib/optimizer/src/Transformer/ServerSideRendering.php Outdated Show resolved Hide resolved
schlessera and others added 2 commits May 15, 2020 18:28
Co-authored-by: Weston Ruter <westonruter@google.com>
…-phpstan-to-level-4

* 'develop' of github.com:ampproject/amp-wp:
  Update dependency @wordpress/scripts to v9.1.0
  Update dependency @wordpress/jest-puppeteer-axe to v1.8.0
  Update dependency @wordpress/url to v2.15.0
  Update dependency @wordpress/plugins to v2.16.0
  Raise PHPStan level to 4 in lib/common (#4686)
  Update dependency uuid to v8
  Update dependency webpack to v4.43.0
  Update dependency eslint-plugin-jest to v23.11.0
  Update dependency eslint-plugin-react to v7.20.0 (#4691)
  Update dependency postcss to v7.0.30 (#4649)
  Update dependency moment to v2.25.3 (#4639)
  Update dependency babel-jest to v26 (#4652)
  Update dependency uuid to v7.0.3 (#4490)
  Update dependency wp-coding-standards/wpcs to v2.3.0 (#4721)
  Update dependency @wordpress/e2e-test-utils to v4.7.0
  Suppress Site Health ICU test if site or home URL is not an IDN (#4698)
  Pin amp-experiment to v0.1 (#4690)
  Disable share icons
  Use amp-embedly-card for Tiktok embeds
  Update dependency terser-webpack-plugin to v2.3.6
@westonruter
Copy link
Member

Travis build does pass but it is not being reported here. See https://travis-ci.org/github/ampproject/amp-wp/builds/687604455

@westonruter westonruter merged commit a23da32 into develop May 15, 2020
@westonruter westonruter deleted the 4380-bump-phpstan-to-level-4 branch May 15, 2020 22:35
westonruter pushed a commit that referenced this pull request May 15, 2020
@amedina
Copy link
Member

amedina commented May 18, 2020

I am assuming that this (and related) PRs were tested in CI properly. No further QA needed.

@westonruter westonruter added this to the v1.6 milestone Jun 2, 2020
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. cla: yes Signed the Google CLA Optimizer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Raise PHPStan level for the amp/optimizer package to 4
5 participants