[D6] Node import SOAP

Manual Review:

Go to http://drupal.org/coding-standards and read about coding standards. In your module commenting, indentation and some other small issues were there.

1. Most of the functions of your module does not have any comment about the purpose of the function.

• node_import_soap_import_import_form_submit($form, &$form_state)
• node_import_soap_import_import_form(&$form_state, $import = NULL)
• node_import_soap_mapping_delete_form_submit($form, &$form_state)
• node_import_soap_mapping_delete_form(&$form_state, $import = NULL, $field = NULL)
• _array_get_values($key, $array)
• _get_functions_fields($importer, $nofields = array())
• _get_fields($type, $nofields = array())
• theme_node_import_soap_function_fields_form($form)
• node_import_soap_mapping_form_submit($form, &$form_state)
• node_import_soap_mapping_form(&$form_state, $import = NULL)
• is_unique($element, &$form_state)
• _foreign_key($importer, $function)
• node_import_soap_function_fields_form_submit($form, &$form_state). and many more..

2. At line number 92, 189, 324. Change '#value' => t(Save) to '#value' => t('Save')

3. At line number 558, you had used $form, which is not passed in the function. Hence it will be treated as undefined variable.

4. At line number 478, 558, 559 indentation is of 4 spaces. It should be of 2 spaces only.

5. Use function array_key_exists for $importer = $form['#parameters'][2]; at line 267, 331, 402, 426 and many more.

#1

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Removing review bonus tag, you have not done any manual review, you just posted the output of an automated review tool. Make sure to read through the source code of the other projects, as requested on the review bonus page.

I also removed the automated review comments from the issue summary.

It appears you are working in the "6.x-1.0" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
The following git branches do not match the release branch pattern, you should remove/rename them. See http://drupal.org/node/1015226

* 6.x-1.0
  remotes/origin/6.x-1.0
  remotes/origin/HEAD -> origin/6.x-1.0

Review of the 6.x-1.0 branch:

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  
  FILE: /home/klausi/pareview_temp/node_import_soap.module
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   333 | ERROR | A cast statement must be followed by a single space
  --------------------------------------------------------------------------------
  

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. project page is too short, see http://drupal.org/node/997024
2. node_import_soap_cron(): why are you overwriting $_GET here? Please add a comment.
3. Your module file is quite large - did you consider to split it up into more useful include files and only keep API functions and hooks in the module file?
4. node_import_soap_invoke_presave(): if you are invoking your own hooks you should do that with module_invoke_all(). And all hooks that your module provides should be documented in an api.php file, see http://drupal.org/node/161085#api_php . Your hooks must be prefixed with your module's name to avoid name collisions with others.
5. node_import_soap_cron(): you do the import on every cron run? I have cron usually configured to run every 5 or 10 minutes, so that would run very often?

Can't say much about the rest of the code, but correctly using hooks is a blocker I guess. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

manual review:

• why do require the include file in the global scope on every single page request? You should only include it if you actually need it from function bodies?

But otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to chx as he might have time to take a final look at this.

Add manual revision.

No objections for more than a week, so ...

Thanks for your contribution, dclavain!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.