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

Make internal methods inaccessible #3229

Merged
merged 1 commit into from
Sep 12, 2019

Conversation

schlessera
Copy link
Collaborator

@schlessera schlessera commented Sep 12, 2019

WP-CLI exposes every public method it encounters in a command object as a new subcommand. Therefore we make the internal methods private again.

As this breaks the current tests, we add a helper function in the tests to invoke these private methods as if they were public. This is a short-term fix for now.

The cleaner long-term solution would be to work on #3077 and get all actual business logic out of the commands. When the commands do nothing more than parsing arguments and formatting outputs, these unit tests become unnecessary here.

Before

Image 2019-09-12 at 10 59 34 AM

After

Image 2019-09-12 at 10 59 50 AM

Fixes #3102

@googlebot googlebot added the cla: yes Signed the Google CLA label Sep 12, 2019
@schlessera schlessera added WP-CLI Bug Something isn't working labels Sep 12, 2019
@kienstra
Copy link
Contributor

kienstra commented Sep 12, 2019

Approved

Hi @schlessera,
Good idea to make these methods private.

Before

Like you mentioned, I also saw that these methods were listed as possible commands:

private-methods-commands

After

With this PR, methods aren't shown as commands:
not-show-as-commds

And the existing commands still work as expected.

Like wp amp validation reset:

cli-reset

...and wp amp validation run:

run-cli

@westonruter westonruter added this to the v1.3 milestone Sep 12, 2019
@westonruter westonruter merged commit 2300413 into develop Sep 12, 2019
@westonruter westonruter deleted the fix/3102-hide-internal-cli-methods-again branch September 12, 2019 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working cla: yes Signed the Google CLA WP-CLI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI integration shows internal methods as commands
4 participants