Allow other modules to alter dump parameters

@@ -49,7 +53,7 @@ function demo_dump_db($filename, $exclud
- * Returns the name of the active database.
+ * Return the name of the active database.

Nice catch! But actually, D7 (HEAD) changed the coding standards for PHPDoc summaries to be in the third person form, as in "Returns foo bar.". :-/

So we might want to revert this change.

@@ -142,7 +163,9 @@ function _demo_dump_table_data($table) {
         if ($query_size + strlen($insert_buffer) > 50000) {
-          $output .= ";\n";
+          // Flush buffer to disc.
+          fwrite($fp, $output . ";\n");
+          $output = '';

If I wouldn't know what we are doing here and why, I would not grok why $output is entirely reset in a loop here. That could use a comment.

@@ -77,7 +77,10 @@ function demo_admin_settings_submit($for
-function demo_manage() {
+/**
+ * Menu callback to display manage snapshots form.
+ */
+function demo_manage_form() {
...
-function demo_manage_submit($form, &$form_state) {
+function demo_manage_form_submit($form, &$form_state) {

Hmmm... We should move those changes to a separate issue, I think.

...oh, scratch that, I see now that it's required for the other changes.

@@ -144,60 +150,62 @@ function demo_dump() {
+/**
+ * Menu callback to display snapshot selection form.
+ */

Ideally, consistently use "Menu callback; Display snapshot selection form."

@@ -235,11 +254,11 @@ function demo_reset($filename = 'demo_si
-    if ($table != $dt_watchdog || $is_version_1_0_dump) {
+    if ($table != $watchdog || $is_version_1_0_dump) {

Do we still need this?

@@ -298,6 +317,8 @@ function demo_reset($filename = 'demo_si
+  // Save time of last reset.
+  variable_set('demo_reset_last', $_SERVER['REQUEST_TIME']);
...
-      variable_set('demo_reset_last', $time);
...
       demo_reset(variable_get('demo_dump_cron', 'demo_site'), FALSE);
-      variable_set('demo_reset_last', $time);

oh, nice! This could finally solve the "cron not always resetting" issue! :)

...shouldn't we use time() here?

...hrm, thinking twice about that, it might as well break cron runs again, because demo_cron() calculates the difference to the last cron run. :-/

@@ -342,6 +363,24 @@ function demo_get_fileconfig($filename =
+  $inc_file = drupal_get_path('module', 'demo') . '/database_' . $engine . '_dump.inc';
+  if (!file_exists($inc_file)) {
+    drupal_set_message(t('@engine support not implemented yet.', array('@engine' => ucfirst($engine))), 'error');
+    return FALSE;
+  }
+
+  require_once $inc_file;

Why aren't we using module_load_include() here?

This review is powered by Dreditor

Thanks for reporting, reviewing, and testing! Committed to all branches.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.