Skip to content

/ core-command

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

Jump to bottom

Change CoreUpgrader::download_package signature #166

Merged
merged 3 commits into from Aug 26, 2020
Merged

Change CoreUpgrader::download_package signature #166

merged 3 commits into from Aug 26, 2020

Conversation

@wojsmol
Copy link
Contributor

@wojsmol wojsmol commented Jul 23, 2020 •
edited

Fixes #167
@wojsmol
Change CoreUpgrader::download_package signature
19a1814
@wojsmol wojsmol requested a review from wp-cli/committers as a code owner Jul 23, 2020
@schlessera
schlessera requested changes Jul 23, 2020
View changes
src/WP_CLI/Core/CoreUpgrader.php Outdated Show resolved Hide resolved
@schlessera
Copy link
Member

@schlessera schlessera commented Jul 23, 2020

How is this called and what is the behavior on older Core versions? I'm wondering whether we might need to support two different versions of the signature to maintain BC...

@wojsmol @schlessera
Update src/WP_CLI/Core/CoreUpgrader.php
d3346bd 
Co-authored-by: Alain Schlesser <[email protected]>
@schlessera
Copy link
Member

@schlessera schlessera commented Aug 26, 2020

The $hook_extra argument actually needs to be forwarded to the 'upgrader_pre_download' filter within the method as well.

@wojsmol
Add $hook_extra to upgrader_pre_download
ed44358
@schlessera
schlessera approved these changes Aug 26, 2020
View changes
@schlessera schlessera added this to the 2.0.11 milestone Aug 26, 2020
@schlessera schlessera merged commit db88c14 into wp-cli:master Aug 26, 2020
3 checks passed
3 checks passed
@github-actions
Lint PHP files
Details
@github-actions
PHPCS
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wojsmol wojsmol deleted the wojsmol:chenge-signature-wp55 branch Aug 26, 2020
TimothyBJacobs added a commit to TimothyBJacobs/core-command that referenced this pull request Nov 5, 2020
@TimothyBJacobs
Fix WP < 5.5 compatibility due to incompatible method signatures.
5c3ddeb 
WordPress 5.5 added a new parameter to `WP_Upgrader::download_package()`. WP-CLI
subclasses the `Core_Upgrader` class to customize this behavior. This causes a
PHP warning on 7.4 and fatal error on 8.0 if the method signatures do not match.

In wp-cli#166 the method signature was adjusted to account for this new parameter,
however this causes the same incompatible signature issue on sites running WP
versions before 5.5.

This commit introduces a trait-based compatibility shim to use the correct method
signature depending on the WP version being run. This is patterned off the
implementation used in the `FeedbackMethodTrait`.
@TimothyBJacobs TimothyBJacobs mentioned this pull request Nov 5, 2020
Closed
@webdock-io webdock-io mentioned this pull request Jan 26, 2021
Closed
@webdock-io
Copy link

@webdock-io webdock-io commented Jan 26, 2021

Please see wp-cli/wp-cli#5494 - This seems related and still an issue, somehow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schlessera schlessera

Assignees
No one assigned
Labels
command:core-update
Milestone
2.0.11
Linked issues

Successfully merging this pull request may close these issues.

WP_CLI\Core\CoreUpgrader::download_package needs to add $hook_extra parameter
3 participants
@wojsmol @schlessera @webdock-io