Skip to content

Conversation

@rrennick
Copy link
Contributor

This PR fixes 3 minor issues with WP CLI command registrations

  • Typos in PHPDoc blocks
  • order synopsis from wp wc generate order <id> to actual implementation wp wc generate order <amount>
  • implement order <amount> default of 100 and change the option to optional

@rrennick rrennick added bug Issue that can be reproduced. needs review labels Oct 27, 2020
@rrennick rrennick self-assigned this Oct 27, 2020
@juliaamosova juliaamosova self-requested a review November 17, 2020 21:53
@juliaamosova
Copy link

@rrennick thanks for working on it!

Typos in PHPDoc blocks

Great, thanks! Looks good.

order synopsis from wp wc generate order <id> to actual implementation wp wc generate order <amount>

What I did here is confirmed that:

  • running wp wc generate orders generates 100 orders by default
  • running wp wc generate orders 30 generates 30 orders

Both generated correct results.

implement order default of 100 and change the option to optional

I am not sure I understand what command should I run here and what result is expected? Would you please clarify?

@rrennick
Copy link
Contributor Author

I am not sure I understand what command should I run here and what result is expected? Would you please clarify?

@juliaamosova The command help said the default was 100 but the implemented default was zero. You tested the default with

running wp wc generate orders generates 100 orders by default

@juliaamosova
Copy link

@juliaamosova The command help said the default was 100 but the implemented default was zero. You tested the default with

running wp wc generate orders generates 100 orders by default

Perfect, thanks for clarifying @rrennick! This is good to go then. The code change looks good to me as well.

@juliaamosova juliaamosova added approved and removed bug Issue that can be reproduced. labels Nov 18, 2020
@rrennick rrennick merged commit a339d93 into master Nov 19, 2020
@rrennick rrennick deleted the fix/order-synopsis branch November 19, 2020 15:47
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.

3 participants