Add --max_num_posts parameter to export command#15
Conversation
|
about the "Warning: limit should be a positive integer" is simply that the current code validate() default values thus making impossible to have a default value outside the scope allowed to the user (what is desirable in case of integer where -1 means |
danielbachhuber
left a comment
There was a problem hiding this comment.
👍 Few minor things to clean up.
src/Export_Command.php
Outdated
| 'end_date' => NULL, | ||
| 'post_type' => NULL, | ||
| 'post_type__not_in' => NULL, | ||
| 'limit' => -1, |
There was a problem hiding this comment.
Could we use NULL here instead of -1? NULL is a more consistent pattern with the rest of the file.
There was a problem hiding this comment.
Based on the failed tests, it appears the warning also needs to accommodate for the default value.
There was a problem hiding this comment.
Current code validation mechanism does not let check_*() functions distinguish between default value and value from user input.
src/WP_Export_Query.php
Outdated
| 'start_date' => null, | ||
| 'end_date' => null, | ||
| 'start_id' => null, | ||
| 'limit' => -1, |
There was a problem hiding this comment.
Ditto comment above about NULL.
src/WP_Export_Query.php
Outdated
| } | ||
|
|
||
| private function limit() { | ||
| if ($this->filters['limit'] > 0) return "LIMIT {$this->filters['limit']}"; |
There was a problem hiding this comment.
This conditional should follow WordPress coding standards.
|
new commits (don't know if you received notification about this) |
|
@drzraf Looks good so far, apart from the breaking test. Travis shows that the This is because |
|
added 6bb2850 |
This brings it more in line with `max_file_size`.
|
Renamed the argument from |
|
Thanks for the pull request, @drzraf ! |
Add --max_num_posts parameter to export command
wp export --limit=<num>