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

Adds argument validations #1989

Merged
merged 2 commits into from
Feb 18, 2021

Conversation

pschoffer
Copy link
Contributor

@pschoffer pschoffer commented Feb 17, 2021

Description

Apart from checking the input, I removed the defaults arguments on the function level, as those were never used and were only misleading (we always send all of the arguments).

Changelog Description

Plugin Updated: Search

Removes minor warning from wp vip-search health validate-contents when some parameters are not set. The correct default values are set both before and after this change.

Checklist

Please make sure the items below have been covered before requesting a review:

  • This change works and has been tested locally (or has an appropriate fallback).
  • This change works and has been tested on a Go sandbox.
  • This change has relevant unit tests (if applicable).
  • This change has relevant documentation additions / updates (if applicable).
  • I've created a changelog description that aligns with the provided examples.

Steps to Test

  1. Running lando wp vip-search health validate-contents Gives you:
Notice: Undefined index: last_post_id in /wp/wp-content/mu-plugins/search/includes/classes/commands/class-healthcommand.php on line 271 [$ /usr/local/bin/wp vip-search health validate-contents] [Automattic\VIP\Search\Commands\HealthCommand->validate_contents(), phar:///usr/local/binvendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98 call_user_func(), WP_CLI\Dispatcher\CommandFactory::WP_CLI\Dispatcher\{closure}(), phar:///usr/local/binvendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451 call_user_func(), phar:///usr/local/binvendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:371 WP_CLI\Dispatcher\Subcommand->invoke(), phar:///usr/local/binvendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:394 WP_CLI\Runner->run_command(), phar:///usr/local/binvendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:1160 WP_CLI\Runner->run_command_and_exit(), phar:///usr/local/binvendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php:23 WP_CLI\Runner->start(), phar:///usr/local/binvendor/wp-cli/wp-cli/php/bootstrap.php:74 WP_CLI\Bootstrap\LaunchRunner->process(), phar:///usr/local/binvendor/wp-cli/wp-cli/php/wp-cli.php:27 WP_CLI\bootstrap(), phar:///usr/local/binphp/boot-phar.php:11 include('phar:///usr/local/binvendor/wp-cli/wp-cli/php/wp-cli.php'), /usr/local/bin/wp:4 include('phar:///usr/local/binphp/boot-phar.php')]
  1. Apply the change - no more Notice.

@pschoffer pschoffer requested a review from a team as a code owner February 17, 2021 13:33
@brandon-m-skinner brandon-m-skinner merged commit 2277017 into master Feb 18, 2021
@brandon-m-skinner brandon-m-skinner deleted the update/fix_validate_content_warning branch February 18, 2021 22:34
@brandon-m-skinner
Copy link
Contributor

r1395-stacks

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.

3 participants