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

Various: Remove pre-PHP 5.6 shims and fallbacks #13465

Merged
merged 18 commits into from
Sep 19, 2019
Merged

Conversation

kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Sep 14, 2019

With Jetpack's minimum PHP version at 5.6, we can clean up various sections of the codebase that provides shims and fallbacks for older versions.

There are two areas not touched by this PR:

  • Debugger library -- there is some encryption that should be better tested vs a wide-ranging PR, and the file structure is setup to include a separate 5.3-era file.
  • Shortcodes -- There is a section where the functionality was designed only with the fallback method, so a replacement should also be better tested.

Changes proposed in this Pull Request:

  • Use the JETPACK__MINIMUM_PHP_VERSION where it can be.
  • Clean up MIME detection. mime_content_type was incorrectly marked deprecated at one time, but it hasn't been so in awhile. No need for the finfo handling.
  • Remove various places where there is a shim/fallback for older PHP verisons.
  • Deprecate a Photon function that duplicated wp_parse_url.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • If you're an Automattician, include a shortlink to the p2 discussion with Jetpack Product here.

Testing instructions:

  • Attempt image uploading via the API (e.g. via Calypso)
  • PHP Unit testing to ensure the Photon tests still work as expected.

Proposed changelog entry for your changes:

  • Maintenance: Remove outdated pre-PHP 5.6 era code.

@kraftbj kraftbj added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Janitorial labels Sep 14, 2019
@kraftbj kraftbj requested review from jeherve and a team September 14, 2019 14:44
@jetpackbot
Copy link

jetpackbot commented Sep 14, 2019

Warnings
⚠️

pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against c935220

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Just a few quick notes.

_inc/lib/class.media.php Outdated Show resolved Hide resolved
_inc/lib/class.media.php Show resolved Hide resolved
_inc/lib/class.media.php Outdated Show resolved Hide resolved
class.json-api-endpoints.php Show resolved Hide resolved
@jeherve jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 14, 2019
functions.photon.php Show resolved Hide resolved
@jeherve jeherve added this to the 7.8 milestone Sep 14, 2019
@kraftbj

This comment has been minimized.

@kraftbj kraftbj force-pushed the fix/min-php-everywhere branch from bcabe42 to a684c06 Compare September 15, 2019 03:15
@kraftbj kraftbj added [Status] In Progress and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 15, 2019
@kraftbj kraftbj force-pushed the fix/min-php-everywhere branch from a684c06 to 582b890 Compare September 16, 2019 15:58
@kraftbj kraftbj force-pushed the fix/min-php-everywhere branch from 582b890 to e9c8a12 Compare September 16, 2019 16:04
@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Sep 17, 2019
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

LGTM! Good idea to create those new functions!

@kraftbj
Copy link
Contributor Author

kraftbj commented Sep 19, 2019

I'll figure out the Fusion bits today so we can get this landed ASAP before the branch grows stale.

@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello kraftbj! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D32920-code before merging this PR. Thank you!

@kraftbj
Copy link
Contributor Author

kraftbj commented Sep 19, 2019

r196720-wpcom r196721-wpcom r196729-wpcom

@kraftbj kraftbj added [Status] Ready to Merge Go ahead, you can push that green button! [Status] Tested on WP.com and removed [Status] In Progress labels Sep 19, 2019
@kraftbj kraftbj merged commit a9959f2 into master Sep 19, 2019
@kraftbj kraftbj deleted the fix/min-php-everywhere branch September 19, 2019 17:16
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 19, 2019
jeherve added a commit that referenced this pull request Sep 20, 2019
@jeherve jeherve removed the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Sep 20, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
jeherve added a commit that referenced this pull request Sep 24, 2019
* Changelog: initial set of changes for 7.8

* Changelog: add #13310

* Changelog: add #13103

* Changelog: add #13426

* Changelog: add #13389

* Changelog: add #13449

* Changelog: add #13461

* Changelog: add #13460

* Changelog: add #13441

* Changelog: add #13454

* Changelog: add #13457

* Changelog: add #13425

* Changelog: add #13473

* Changelog: add #13355

* Changelog: add #13451

* Changelog: add #13358

* Changelog: add #13464

* Changelog: add #13416

* Changelog: add #13494

* Changelog: add #13465

* Changelog: add #13424

* Changelog: add #13432

* Changelog: add #13471

* Changelog: add 7.7.2 entry

* Changelog: add #13446

* Add more testing elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants