unpack method on Controller, test for loading exportable from database

+++ b/lib/Drupal/ctools/DatabaseExportableController.phpundefined
@@ -335,4 +345,33 @@ class DatabaseExportableController extends ExportableControllerBase {
+        $exportable->$field = empty($this->schema[$field]['serialize']) ? $data[$field] : unserialize($data[$field]);

I think this would be more clear if it were !empty() and the unserialize call came first

+++ b/lib/Drupal/ctools/DatabaseExportableController.phpundefined
@@ -335,4 +345,33 @@ class DatabaseExportableController extends ExportableControllerBase {
+        $join_schema = ctools_export_get_schema($join['table']);

Move this inside the conditional.

+++ b/lib/Drupal/ctools/DatabaseExportableController.phpundefined
@@ -335,4 +345,33 @@ class DatabaseExportableController extends ExportableControllerBase {
+            $info = $join_schema['fields'][$field];

I realize this is how it was in _ctools_export_unpack_object(), but no reason to define $info as an extra variable here since it's only used once below.

+++ b/lib/Drupal/ctools/DatabaseExportableController.phpundefined
@@ -335,4 +345,33 @@ class DatabaseExportableController extends ExportableControllerBase {
+            $exportable->$field = empty($info['serialize']) ? $data->$field : unserialize($data->$field);

Isn't $data an array here? Also probably best to switch the order of the ternary as above

+++ b/lib/Drupal/ctools/ExportableBase.phpundefined
@@ -48,14 +48,11 @@ class ExportableBase implements ExportableInterface {
+  public function unpack($data) {

Add a junk @todo please :)

+++ b/tests/ctools_export_test/ctools_exportable.testundefined
@@ -52,6 +52,23 @@ class CToolsExportableTestCase extends WebTestBase {
+    $loaded_export = $controller->load('database_test');

It'd be marginally clearer to move this below the $expected_export definition, I'd think.

Cross post.

Committed! http://drupalcode.org/project/ctools.git/commit/7a7b198

We committed this with @todo as a docblock? :P

Reopening for followup. :)

We already talked about this on IRC -- xjm is keeping us honest.

Tim and I were trying to be agile -- one of the things I hate the most is that if I document a design that I'm still trying to perfect, I am more likely to fail to notice that I changed a parameter and didn't adjust the documentation. On the other hand, @todo is impossible to miss. Thus, I thought it would be the most efficient use of time and ultimately lead to the most correct code to *really obviously* not be documented so that it can be done properly once we are confident the design is what we want.

And the reality proved it -- we changed method names and parameters a lot while working on it -- and it was even hard to make sure the interfaces matched.

I do think we probably want a single documentation issue, though, because MOST of the doc will probably come in a single pass. It may miss things, and we can do followup issues on it. Now that the design is pretty well solidified, a documentation pass should be an early priority.

Everyone okay with that approach? If so, I'd recommend opening a documentation issue, linking it here, and putting this one back to its previous state.