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

Use --max_file_size=-1 to avoid splitting export files #12

Merged
merged 5 commits into from Sep 29, 2017

Conversation

4 participants
@drzraf
Contributor

drzraf commented Aug 18, 2017

Fixes #8

@drzraf drzraf changed the title from issue#8: --max_file_size=0 does not split the export to --max_file_size=0 does not split the export Aug 18, 2017

@miya0001 miya0001 self-requested a review Aug 18, 2017

@miya0001

I am feeling -1 is better than 0.
It will be similar with the php.ini.

What do you think about it?

@miya0001

This comment has been minimized.

Show comment
Hide comment
@miya0001

miya0001 Aug 18, 2017

Member

Can you add tests too?

Member

miya0001 commented Aug 18, 2017

Can you add tests too?

@drzraf

This comment has been minimized.

Show comment
Hide comment
@drzraf

drzraf Aug 21, 2017

Contributor

PR updated (not sure about my new behat's test, couldn't run the tests yet).
-1 is now the documented value, but 0 has the same (undocumented) effect since it seems more logical to me (and may be a sane behaviour for possible future reuse)

Contributor

drzraf commented Aug 21, 2017

PR updated (not sure about my new behat's test, couldn't run the tests yet).
-1 is now the documented value, but 0 has the same (undocumented) effect since it seems more logical to me (and may be a sane behaviour for possible future reuse)

Show outdated Hide outdated features/export.feature Outdated
Show outdated Hide outdated src/Export_Command.php Outdated
@drzraf

This comment has been minimized.

Show comment
Hide comment
@drzraf

drzraf Aug 31, 2017

Contributor

new commits (don't know if you received notification about this)

Contributor

drzraf commented Aug 31, 2017

new commits (don't know if you received notification about this)

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Sep 19, 2017

Member

@drzraf Is this completed to your satisfaction?

Member

danielbachhuber commented Sep 19, 2017

@drzraf Is this completed to your satisfaction?

@danielbachhuber danielbachhuber changed the title from --max_file_size=0 does not split the export to Use --max_file_size=-1 to avoid splitting export files Sep 19, 2017

@danielbachhuber danielbachhuber added this to the 1.0.3 milestone Sep 19, 2017

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Sep 29, 2017

Contributor

A problem is that WP_EXPORT_NO_SPLIT isn't getting defined if KB_IN_BYTES is defined, which is the case for modern WPs but not for older ones such as 3.7.11. This isn't visible currently on Travis, only locally in error_log. Another problem is that max_file_size is getting multiplied by MB_IN_BYTES, with no check for WP_EXPORT_NO_SPLIT.

A problem with the tests is that they need a database > 15MB to check the no-split versus the default.

I made changes (eventually) to address these in edit gitlost#2 gitlost#3, which I can push if desired.

Contributor

gitlost commented Sep 29, 2017

A problem is that WP_EXPORT_NO_SPLIT isn't getting defined if KB_IN_BYTES is defined, which is the case for modern WPs but not for older ones such as 3.7.11. This isn't visible currently on Travis, only locally in error_log. Another problem is that max_file_size is getting multiplied by MB_IN_BYTES, with no check for WP_EXPORT_NO_SPLIT.

A problem with the tests is that they need a database > 15MB to check the no-split versus the default.

I made changes (eventually) to address these in edit gitlost#2 gitlost#3, which I can push if desired.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Sep 29, 2017

Member

I made changes (eventually) to address these in edit gitlost#2 gitlost#3, which I can push if desired.

Please do!

Member

danielbachhuber commented Sep 29, 2017

I made changes (eventually) to address these in edit gitlost#2 gitlost#3, which I can push if desired.

Please do!

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Sep 29, 2017

Contributor

which I can push if desired.

Okay, I don't know how to do this on this branch, even after reading https://help.github.com/articles/committing-changes-to-a-pull-request-branch-created-from-a-fork/, so I'll open a new PR.

Contributor

gitlost commented Sep 29, 2017

which I can push if desired.

Okay, I don't know how to do this on this branch, even after reading https://help.github.com/articles/committing-changes-to-a-pull-request-branch-created-from-a-fork/, so I'll open a new PR.

@gitlost gitlost referenced this pull request Sep 29, 2017

Merged

Drzraf no max file size #21

@danielbachhuber danielbachhuber merged commit d18fb60 into wp-cli:master Sep 29, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment