@danielbachhuber Please give this a spin when you get the chance.

Will do early next week

On Mon, Jun 17, 2013 at 2:39 AM, Cristi Burcă notifications@github.com
wrote:

@danielbachhuber Please give this a spin when you get the chance.

Reply to this email directly or view it on GitHub:
#525 (comment)

I find myself overusing git rebase too. It makes the history cleaner, but it's a fake history. Plus, you lose the build results for intermediate commits.

Let's make a deal pact: no more rebasing of branches. Instead, when a branch gets dirty outdated, we just merge master into it and fix the conflicts.

If a branch gets really outdated, we just abandon it and start a new one, with a different name.

Let's make a deal: no more rebasing of branches. Instead, when a branch gets dirty, we just merge master into it and fix the conflicts.

Works for me. Hopefully this leads to smaller branches too, so there aren't a bunch of merge commits clogging the history :)

I took a brief look at this. It seems like a good improvement, but I'm not yet sold on it as a great improvement.

Some thoughts:

• Does it properly handle large exports (e.g. 200MB+)? We had some object cache reset hack previously to avoid memory leaks that's been removed.
• Seems much faster. Do we need the CLI progress indicators back?
• I included the CLI arguments in the filename originally so you could easily create multiple export files on a single day without worrying about them overwriting each other. It would be nice to restore those.
• Not sure how I feel about removing file_item_count and replacing it with splitting based on file size.

cc @nb

Does it properly handle large exports (e.g. 200MB+)? We had some object cache reset hack previously to avoid memory leaks that's been removed.

No idea; need to test.

Seems much faster. Do we need the CLI progress indicators back?

I'm not sure how easy it would be to add them back; we'd have to fork even more of the new export API.

I included the CLI arguments in the filename originally so you could easily create multiple export files on a single day without worrying about them overwriting each other. It would be nice to restore those.

I don't think including the CLI arguments verbatim was a good idea to start. We could add their md5 hash though.

Not sure how I feel about removing file_item_count and replacing it with splitting based on file size.

I think it makes sense; the file size is what you really care about when splitting.

we'd have to fork even more of the new export API.

Can we instead ask Nikolay to include actions within the export API that achieve the end results we want? I haven't done a diff to see what we've changed, but my preference would be no forking at all.

I don't think including the CLI arguments verbatim was a good idea to start. We could add their md5 hash though.

Care to explain? Including them verbatim achieves two things:

1. Tells me which export the file corresponds with.
2. Prevents a second export from overwriting the first.

An md5 hash only achieves the second.

It's not a good idea because the arguments can get really long and might contain unusual characters (for a file name).

I understand it's convenient, but it doesn't justify the potential headaches.

It's not a good idea because the arguments can get really long and might contain unusual characters (for a file name).

Let that become a problem when it becomes a problem. I was sanitizing the filename previously.

Copying the relevant code chunk here, for convenience:

$append = array( date( 'Y-m-d' ) );
foreach( array_keys( $args ) as $arg_key ) {
    if ( $defaults[$arg_key] <> $args[$arg_key] && 'post__in' != $arg_key )
        $append[]= "$arg_key-" . (string) $args[$arg_key];
}
$file_name_base = sanitize_file_name( $sitename . 'wordpress.' . implode( ".", $append ) );

Still not sold; it would be cleaner to allow the user to specify an "ID" for a batch of export files. For example:

wp export --batch-id=events-for-x --post_type=event

which would generate:

testground.wordpress.2013-06-events-for-x.0.xml
testground.wordpress.2013-06-events-for-x.1.xml
testground.wordpress.2013-06-events-for-x.2.xml

Or even allow them control of the entire filename format:

wp export --filename-format='events-for-x-{site}-{date}-{nr}.xml' --post_type=event

I guess my beef with the old code is that 1) the filenames generated by default are too dynamic and 2) you can't change the default.

I am cool with adding actions and filters, I almost haven't thought how it will be extended, so I almost haven't added actions or filters.

Added 'wp_export_new_file' hook: 743092d

Suggestions for better name and/or better placement (while achieving similar functionality) welcome.

Guys, I'm going to merge this pretty much as-is.

My hunch is that people will be a lot more likely to submit incremental improvements once they see that the code is relatively clean.