Skip to content

Conversation

BhargavBhandari90
Copy link
Contributor

@BhargavBhandari90 BhargavBhandari90 commented May 5, 2019

Added strict mode for taxonomy list command.
Default will be --no-strict

Fixes #181.

@BhargavBhandari90 BhargavBhandari90 requested a review from a team as a code owner May 5, 2019 09:12
@swissspidy
Copy link
Member

What would strict mean here? What issue does it solve?

@BhargavBhandari90
Copy link
Contributor Author

@swissspidy I am working on this issue: #181

@swissspidy swissspidy changed the title Issue #181 : Add strict/no-strict mode for taxonomy list Add strict/no-strict mode for taxonomy list May 5, 2019
Copy link
Member

@schlessera schlessera left a comment

Choose a reason for hiding this comment

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

I added a few comments in-line into the PR for changes to make.

Also, please add one or more functional tests to cover this new flag.

@schlessera
Copy link
Member

Great, code looks good. Can you add one or more functional tests to make sure it also works as expected?

@BhargavBhandari90
Copy link
Contributor Author

@schlessera . I'll be adding the functional tests over the weekend. Thanks 👍

Copy link
Member

@schlessera schlessera left a comment

Choose a reason for hiding this comment

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

Two CS tweaks only.

@thrijith
Copy link
Member

@schlessera for this, does it need a check here in FeatureContext ?

@schlessera
Copy link
Member

Oh, thanks @thrijith, somehow I had looked at the wrong test output here... 🙃
I'll check what the issues are...

@BhargavBhandari90
Copy link
Contributor Author

@schlessera Somehow the test is failing. I am not sure about that. May be it's related to what @thrijith was saying...?

@schlessera schlessera added this to the 2.2.1 milestone Jan 13, 2022
@schlessera schlessera merged commit d7d08b0 into wp-cli:master Jan 24, 2022
@grappler
Copy link

Does the new flag not need to be documented in the function header?

@schlessera
Copy link
Member

@grappler Yes, indeed! That will be for the next release then, as I just pushed 2.6.0. :)

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.

Use get_object_taxonomies() for taxonomy list by post type object
6 participants