Conversation
| if ( isset( $plugin_update_info->requires ) && version_compare( $wp_version, $requires, '>=' ) ) { | ||
| $reason = "This update requires WordPress version $plugin_update_info->requires, but the version installed is $wp_version."; | ||
| } elseif ( ! isset( $update_info['package'] ) ) { | ||
| } elseif ( ! isset( $plugin_update_info->package ) ) { |
There was a problem hiding this comment.
$update_info does not exist at this point.
Introduced in #451
There was a problem hiding this comment.
🤦♀️ yes that is a typo since these are similarly named. I should set up PHPStan (!)
There was a problem hiding this comment.
Good thing that I am working on a unified setup :-)
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| * @param array $assoc_args | ||
| */ | ||
| public function activate( $args = array() ) { | ||
| public function activate( $args, $assoc_args = [] ) { |
There was a problem hiding this comment.
This changed $args from optional to required. Was this intended?
There was a problem hiding this comment.
Yes, because it is actually required as per the synopsys, and WP-CLI always passes both arguments to commands anyway. I've never seen $args or $assoc_args be null actually, so should even change this to ( $args, $assoc_args ).
src/Theme_Mod_Command.php
Outdated
| * $ wp theme mod set background_color 000000 | ||
| * Success: Theme mod background_color set to 000000. | ||
| * | ||
| * @param string[] $args |
There was a problem hiding this comment.
Different types then for activate() above. We should probably define and stick to a consistent type hint for command methods.
|
|
||
| abstract protected function install_from_repo( $slug, $assoc_args ); | ||
|
|
||
| abstract public function activate( $args, $assoc_args = [] ); |
There was a problem hiding this comment.
Should probably stick to consistent command signature as well.
There was a problem hiding this comment.
So $assoc_args is sometimes optional because the class is extended. The method is sometimes called with 1 parameter from within the sub class itself.
Co-authored-by: Alain Schlesser <alain.schlesser@gmail.com>
|
@wp-cli/committers would appreciate a review here to bring the whole PHPStan work to a close :) thx! |
mrsdizzie
left a comment
mrsdizzie
left a comment
There was a problem hiding this comment.
Looks ok to me, thanks for all this work!
3 participants
Mostly doc block improvements, but also fixes a regression from #451