wc-smooth-generator
wc-smooth-generator copied to clipboard
adding count parameter
As detailed in issue https://github.com/woocommerce/wc-smooth-generator/issues/76 , a count parameter would be beneficial so that scalable import events could be run from the WordPress cron infrastructure.
All Submissions:
- [x] Have you followed the Contributing guidelines?
- [x] Does your code follow the WordPress' coding standards?
- [x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
Changes proposed in this Pull Request:
Closes #76 .
How to test the changes in this Pull Request:
- Merge PR
- Schedule an event from console
wp cron event schedule wc_smooth_generate_object now --type=order --count=10
- Check that 10 orders have been created
Other information:
- [ ] Have you added an explanation of what your changes do and why you'd like us to include them?
- [ ] Have you written new tests for your changes, as applicable?
- [ ] Have you successfully run tests with your changes locally?
Changelog entry
Added
count
parameter to thewc_smooth_generate_object
method
FOR PR REVIEWER ONLY:
- [x] I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.
This looks like a powerful, efficient suggestion to me. Thanks for contributing!
I'd only suggest using a do while
instead of while
, as we guarantee that the generation executes at least once:
function wc_smooth_generate_object( $type, $count = 1 ) {
$i = 0;
do {
$i ++;
switch ( $type ) {
case 'order':
Generator\Order::generate();
break;
case 'product':
Generator\Product::generate();
break;
case 'customer':
Generator\Customer::generate();
break;
case 'coupon':
Generator\Coupon::generate();
break;
default:
return false;
}
} while ( $i < intval( $count ) );
return false;
}
@ovidiul would you mind committing the suggestions above, so I can approve this PR and forward this internally to be merged?
@Luc45 changes done, thanks for considering this.
Thanks for the change, @ovidiul!
Just out of curiosity, a couple of PHP core developers once warned me about doing computation inside a statement, because the inner workings of PHP cannot guarantee the order of operations at all times, so I usually pre-compute it:
$i = 0;
do {
$i ++;
} while ( $i < intval( $count ) );
Instead of:
$i = 0;
do {
} while ( $i++ < intval( $count ) );
But I have tested your version and it works fine with all PHP versions, so this seems to be perfectly fine in this case.
I'll be approving this PR now and forwarding this internally to see if we can get this merged soon.
Hi @ovidiul,
Apologies not getting to this sooner. It looks like there are some conflicts now. Would you mind resolving those so that we can give this a re-review?
Hi @jonathansadowski conflict is now resolved.
Thanks
Thanks @ovidiul — merging now 😄