Thanks for the review and the patch. I applied the patch but then the files weren't being produced in the correct place or with the correct names. Also the produced files weren't being listed on the import page. So I reverted the patch and have applied and tested your suggestions manually one at a time. Please note this module currently exports and imports taxonomy terms and nodes of type page. When these are imported they are set to have the same ID's as the original data to make sure links between terms and nodes all link correctly. I have a development branch where the module exports and imports all content types - and I'll merge this branch in later. The current module is fully functional and is already extremely useful for synhronizing taxonomy terms between Drupal sites. Here are the results from manually applying all your points: 1.) http://drupal.org/coding-standards#array place closing brackets at new line for me its easy da read and avoid problems will add new array element KB - Agreed - ALL CHANGED. 2.) array elements shoulb be idneted with 2 spaces KB - Agreed - ALL CHANGED. 3.) i dont like debug stuff in commited code, but this is my personal opinion if ($debug) { Some errors: KB - Agreed - ALL REMOVED. 1.) function data_export_import_import_form() file: data_export_import.module line: 197 $directory = "sites/default/files/data_export_import/pages"; remove this line, ovoid hard coded pathes not every one have this module in sites/default KB - Agreed - ALL HARD CODED PATHS CHANGED TO PICK UP FILES DIRECTORY VIA API. 2.) files missing profiles/pages.inc profiles/taxonomy_terms.inc KB - Can't see what this is referring to. 3.) avoid include_once use module_load_include() insted KB - Agreed - ALL CHANGED. 4.) function data_export_import_export_form_submit() file: data_export_import.module line: 320 $dataset_file = data_export_import_export_articles() call a not declared function KB - Agreed - REMOVED. 5.) include filename in t() in other languages perhaps it not have to be on the end af string check_plain(t("The Articles dataset was output to the file:") . $dataset_file) t('The Articles dataset was output to the file: @dataset_file', array('@dataset_file', $dataset_file)) KB - Agreed - ALL CHANGED. NB - The format needs to be: drupal_set_message(t('The Pages dataset was output to the file: %dataset_file', array('%dataset_file' => $dataset_file))); 6.) function data_export_import_import_form_submit() file: data_export_import.module line: 403 trigger buttons with not exist make no sens if ($form_state['clicked_button']['#post']['taxonomy_terms'] != 'none') { how to trigger button clicks within drupal: http://api.drupal.org/api/drupal/includes--form.inc/function/_form_butto... KB - THIS IS FINE - IF NO DATA FILE SELECTED THEN DO NOTHING - OTHERWISE IMPORT THE SELECTED FILE. 7.) function data_export_import_export_pages() file: page.inc line: 26 if it is an object, use it as object KB - THIS WAS HEAVILY TESTED. When the objects were unserialized they would show as different even if the data in the objects was identical. Many different combinations were tried until the working code was finished. 8.) function data_export_import_export_pages() file: page.inc line: 44 if you will do this with an content type, with more as 100 nodes, you will run out of php max execution time! it is realy not usable to save all nodes to same file http://api.drupal.org/api/drupal/includes--form.inc/group/batch/7 KB - NOT CURRENTLY AN ISSUE. This has been tested with 5,000 nodes and the content is generated in less than one second. Also, the data file was 8MB only. If needed this can be changed to use the Batch API at a later date. A check could be put in to stop the export if there are more than 5,000 nodes. This was also tested with three vocabularies with 5,000 terms in each and worked file. 9.) function data_export_import_export_pages() file: page.inc line: 60 avoid serializie, use var_export or json_encode insted why use serializie when there is no need to recreate classes, it is overkill json_encode() or var_export() ar mutch faster. The featuresm odule an some other prever var_export() if you are afraid of securety issues or white page dies, use json_encode() KB - NOT CURRENTLY AN ISSUE. The formatting of the data being put into the file was heavily tested. Basically, the data is built up as an array - this is serialized - output to a file. When importing the data is unserialized and then the array is available for use. json_encode() was tested as it would have been the preferred choice but it was found not to work. 10.) function data_export_import_export_pages() file: page.inc line: 156 dont compare arryas to each other. better compare json or serialized strings. Bett hte md5`s of them KB - NOT CURRENTLY AN ISSUE. The serialized or md5's arrays can not be compared directly as there are some changes in the data which needs to be ignored. I.e. - the data needs to be unserialized into an array - and then certain values (such as create datatimes etc) then need to be unset before the comparison is made. 11.) function data_export_import_export_pages() file: page.inc line: 171 t() dont need ony check_plain() bevor http://api.drupal.org/api/drupal/includes--common.inc/function/t/6 KB - Agreed - ALL CHANGED. 12.) function data_export_import_export_pages() file: page.inc line: 171 if a drupal_set_message() is an, error, dont prefix it with "ERROR -", set second parameter "error" KB - Agreed - ALL CHANGED. 13.) function data_export_import_export_pages() file: page.inc line: 205 count($file_content_as_array['nodes'])==0 better use if(empty($file_content_as_array['nodes'])) for better performance and error handling KB - Agreed - ALL CHANGED. 14.) function data_export_import_export_taxonomy_terms() file: taxonmy_terms.inc line: 35 directly use vid is dangerous, cause perhaps them are not equal on diferent machines better use vocabular machine name for identify specizialy in the case when you have several developers for one project /** * Get a taxonomy object by its name * @param string $name The name of the taxonomy * @return stdClass The vocabulary object */ function get_taxonomy_by_name($name) { $vocabularies = taxonomy_get_vocabularies(); foreach($vocabularies as $vid => $vocabulary) { if ($vocabulary->name == $name) { return $vocabulary; } } return null; } KB - NOT CURRENTLY AN ISSUE. An important point about this module is that the vocabularies and terms and nodes end up with the same ID's as when they were exported. This means that the data is an exact match and that all cross references work as expected. 15.) file: data_export_import.info the dependencies are missing KB - Agreed - CHANGED. 15.) $synonym_list .= $synonym . "\n"; is faster to read than $synonym_list = $synonym_list . $synonym . "\n"; questions KB - Agreed - ALL CHANGED. 1.) Overrides the standard drupal_write_record() function. function data_export_import_drupal_write_record_via_insert_with_defined_id() why you have had to do that? KB - NOT CURRENTLY AN ISSUE. This is core to the module. When creating a node from data in the dataset it creates a node with the same ID. TO make this work I had to 'override' two Drupal API functions. This is because the standard functions want to create nodes with new ID's which are the next ID in sequence. 2.) Why is data_export_import_taxonomy_get_tree_with_reset() better than http://api.drupal.org/api/drupal/modules--taxonomy--taxonomy.module/func... KB - NOT CURRENTLY AN ISSUE. This adds a (missing) reset parameter to the function. This is needed because we are using this function repeatedly during the import. If the data was not refreshed from the database then stale (cached) data would be used and this would produce errors. 3.) did you have tested the taxonomy import / export with transalated terms? KB - THANKS FOR POINTING THIS OUT Although I managed to get the terms with 'per language terms' localization working I'm not sure it was a complete solution. There I've put in code to detect localization and warn the user that the code does not handle localized data yet. If I can get this initial module accepted my employer may be OK to fund further work in this area later. WATCHDOG ERROR REPORTING Thanks for the code RE reporting to Watchdog - unfortunately it did not work but I've kept it for reference. At this point the file list has just been created by the user opening the form - so the the file is not likely to have dispeared in the few seconds between loading the form and running the import. Also, I've not included checking of the format of the file - this module is designed to be used by admins and developers for transferring data between Drupal sites. If it was to be used by normal users then checking would be introduced. Please note - there is further development which can be carried out - but this module is certainly large enough and useful enough to be included as a full Drupal module. Once this review process is complete I can carry out further work. The next code to be brought in is to get the module to handle all content types including those with CCK fields and file fields.