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

Add skips arg to Utils\report_batch_operation_results(). #4429

Merged
merged 2 commits into from Oct 10, 2017

Conversation

2 participants
@gitlost
Contributor

gitlost commented Oct 7, 2017

See wp-cli/media-command#48 (review)

Adds $skips arg to Utils\report_batch_operation_results() to report the number of items skipped. Also adds reporting of the number of items that failed.

The default has been to set to null which suppresses the reporting of failed items, for back compat with current behaviour. If temporarily breaking the tests for the commands that test for this (entity-command, extension-command, media-command and widget-command) is okay, then the default should be zero instead.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Oct 7, 2017

Member

The default has been to set to null which suppresses the reporting of failed items, for back compat with current behaviour.

I'd actually think this the expected behavior: don't show any message about skips unless a value is explicitly provided.

I've updated the PHPDoc accordingly.

Member

danielbachhuber commented Oct 7, 2017

The default has been to set to null which suppresses the reporting of failed items, for back compat with current behaviour.

I'd actually think this the expected behavior: don't show any message about skips unless a value is explicitly provided.

I've updated the PHPDoc accordingly.

@danielbachhuber danielbachhuber added this to the 1.4.0 milestone Oct 7, 2017

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Oct 7, 2017

Contributor

don't show any message about skips

If null it won't show any message about the number of fails, even if $failures is non-zero. This is the BC behaviour.
If null or zero if won't show any message about skips.

Contributor

gitlost commented Oct 7, 2017

don't show any message about skips

If null it won't show any message about the number of fails, even if $failures is non-zero. This is the BC behaviour.
If null or zero if won't show any message about skips.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Oct 10, 2017

Member

@gitlost Ah, I understand now. I think the current implementation looks 👍

Member

danielbachhuber commented Oct 10, 2017

@gitlost Ah, I understand now. I think the current implementation looks 👍

@danielbachhuber danielbachhuber merged commit 506b9c8 into master Oct 10, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@danielbachhuber danielbachhuber deleted the report_batch_skips branch Oct 10, 2017

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