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

Rename wp-cli commands #3028

Merged
merged 8 commits into from
Oct 25, 2022
Merged

Rename wp-cli commands #3028

merged 8 commits into from
Oct 25, 2022

Conversation

MARQAS
Copy link
Contributor

@MARQAS MARQAS commented Sep 27, 2022

Description of the Change

This PR is used to rename the WP-CLI commands as requested in #2981

Closes #2981

How to test the Change

  • Run the old WP-CLI commands, they will give a deprecation warning along with the expected output
  • Run the renamed commands, they will give the expected output

Changelog Entry

Added - get-cluster-indices, get-indices, sync, clear-sync, get-ongoing-sync-status, get-last-cli-sync, stop-sync WP-CLI commands
Deprecated - get-cluster-indexes, get-indexes, index, clear-index, get-indexing-status, get-last-cli-index, stop-indexing WP-CLI commands

Credits

Props @MARQAS

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@MARQAS MARQAS self-assigned this Sep 27, 2022
@MARQAS MARQAS marked this pull request as ready for review September 27, 2022 21:14
@MARQAS MARQAS added this to the 4.4.0 milestone Sep 27, 2022
Copy link
Member

@felipeelia felipeelia left a comment

Choose a reason for hiding this comment

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

@MARQAS I've been thinking about this one and before we merge it there is a couple of changes we will need to make here:

  1. To avoid breaking someone else's code that can be extending our Command class, instead of a whole new class, let's make DeprecatedCommand a trait and include it in the regular Command class. This way we keep methods in a separate file but they will still be available to others.
  2. The _deprecated_function function does not output a warning to the user. Let's keep it there but also make a call to WP_CLI::warning and add a notice to the command doc. Examples:
    https://github.com/wp-cli/language-command/blob/8e25528afc90b709aaebba429752a86d12c500d1/src/Core_Language_Command.php#L334
    https://github.com/wp-cli/scaffold-command/blob/4ae0066a64db0b93b4f923554cb8ab2b921e16c9/src/Scaffold_Command.php#L207

@felipeelia
Copy link
Member

felipeelia commented Oct 25, 2022

After 93e47a3, this still needs to be done (all in DeprecatedCommand.php):

  1. Remove all @alias from the commands
  2. Add WP_CLI::warning() calls to each command
  3. Update the documentation of each command adding preceding the main desc. with DEPRECATED and adding the alternative to the long description
  4. Test it (manual testing should be enough for this, so we don't bloat our test suites with deprecated stuff)
  5. Fix the documentation of clear_index to include $args and $assoc_args
  6. Fix git conflicts

@MARQAS MARQAS assigned felipeelia and unassigned MARQAS Oct 25, 2022
@felipeelia felipeelia merged commit 4822957 into develop Oct 25, 2022
@felipeelia felipeelia deleted the feature/2981 branch October 25, 2022 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename WP-CLI commands
2 participants