Lets create a phpexporter class

CommentFileSizeAuthor
#19 1624952-php-exporter-19.patch6.43 KBdamiankloip
#16 1624952-php-exporter-16.patch5.33 KBdawehner
#12 1624952-php-exporter-11.patch5.27 KBdawehner
#12 interdiff.txt1.19 KBdawehner
#9 1624952-php-exporter-9.patch5.17 KBdawehner
#4 1624952-php-exporter-4.patch5.11 KBdawehner
#3 1624952-php-exporter-3.patch5.1 KBdawehner
#2 1624952-php-exporter-2.patch3.91 KBdawehner
#1 1624952-php-exporter-1.patch3.86 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 1624952-php-exporter-1.patch3.86 KB

Just some tracking of the work.

The export tests are running fine, though the import tests are failing completely at the moment.

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new 1624952-php-exporter-2.patch3.91 KB

Managed to get it working.

PhpExporterTest::exporterData() could be reused by the yaml exporter.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 1624952-php-exporter-3.patch5.1 KB

Make it easier for a potential yaml implementation.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 1624952-php-exporter-4.patch5.11 KB

Lets use assertIdentical, based on chx idea.

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +VDC

.

dawehner’s picture

dawehner
Primary language German
commented

.

merlinofchaos’s picture

merlinofchaos commented

I think we should use drupal_var_export over ctools_var_export unless drupal_var_export is missing something key.

merlinofchaos’s picture

merlinofchaos commented
Version: 7.x-1.x-dev » 8.x-1.x-dev
dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 1624952-php-exporter-9.patch5.17 KB

Add some documentation and improve the getInfo

merlinofchaos’s picture

merlinofchaos commented

Still needs #7.

Also I learned today we should use "Defines" instead of "Definition of" in the doxy.

aspilicious’s picture

aspilicious commented

Merlin that is incorrect for file documentation headers for PSR-0 code.

See: http://drupal.org/node/1353118

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new interdiff.txt1.19 KB
new 1624952-php-exporter-11.patch5.27 KB

I'm confused about your last comment, so i asked aspilicious, he pointed me to http://drupal.org/node/1353118 and basically every instance in core uses "Definition of".

Added some documentation on the class itself.

In general i'm sorry i didn't saw both your latest comments in both issues.

damiankloip’s picture

damiankloip commented
Title: Create a exporter which uses php as export format » Implement a PHP Exporter class
damiankloip’s picture

damiankloip commented
Status: Needs review » Needs work 
+++ b/lib/Drupal/ctools/PhpExporter.phpundefined
@@ -0,0 +1,33 @@
+* Definition of Drupal\ctools\PhpExporter.

Not sure about how this is phrased. How about just 'Defines a ctools PHP exporter.'

+++ b/lib/Drupal/ctools/PhpExporter.phpundefined
@@ -0,0 +1,33 @@
+   * Implements ExporterInterface::export().

I think this needs to have the full namespace, Drupal\ctools\ExporterInterface

+++ b/lib/Drupal/ctools/PhpExporter.phpundefined
@@ -0,0 +1,33 @@
+   * Implements ExporterInterface::export().

Same here.

Tests look like a good start too!

aspilicious’s picture

aspilicious commented

"Definition of" is fine in @file header. I stronly recommend readng the docs dawhener linked in #12.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 1624952-php-exporter-16.patch5.33 KB

Fixed documentation of export/import.

aspilicious’s picture

aspilicious commented
Status: Needs review » Needs work 
+++ b/lib/Drupal/ctools/Tests/ExporterBaseTest.phpundefined
@@ -0,0 +1,84 @@
+* @file
+* Definition of Drupal\ctools\Tests\ExporterBaseTest.

I think this is missing a space before the *

damiankloip’s picture

damiankloip commented

@aspilicious, that isn't the part I wanted to highlight, my mistake. I am ok with what should be in a @file header, I don't think 'Defines a ctools PHP exporter.' would make sense there.

@dawehner: the line I was referring to was: "Defines a ctools exporter which uses PHP to dump the config."

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new 1624952-php-exporter-19.patch6.43 KB

Here is an updated patch, with aspilicious' observation in too.

I have also removed the current ExporterPHP.php file, and changed the default info array in ctools_exportable_get_info() to use PhpExporter instead of ExporterPHP.

I think we might be good to go with this now.

merlinofchaos’s picture

merlinofchaos commented
Status: Needs review » Reviewed & tested by the community
damiankloip’s picture

damiankloip commented

#19: 1624952-php-exporter-19.patch queued for re-testing.

damiankloip’s picture

damiankloip commented
Status: Reviewed & tested by the community » Fixed

Committed, http://drupalcode.org/project/ctools.git/commit/4419b52

Automatically closed -- issue fixed for 2 weeks with no activity.