jonathan_hunt [jonathanhunt]

1. See the Drupal coding standards to understand how a module code should be written; there is a part of the coding standards that reports which functions a module should call.

2.   static $_digitalnz_searched = FALSE;
   

   Static variables doesn't need to be prefixed with the module name; that is how global variables used by a module should be named.

1.     '#title' => t('DigitalNZ API URL'),
       '#description' => t('URL of DigitalNZ API including version but not response format, e.g. '. l(DIGITALNZ_API_URL, DIGITALNZ_API_URL)),
   

   Together t() you should use url(), as reported in the documentation for t().

2. The coding standards report also which functions should be called from a module; the report is for a specific group of functions, but it is also generally valid.

3.       if (isset($sort) && preg_match('/^([a-z0-9_]+ (asc|desc)(,)?)+$/i', $sort)) {
           $sort = $sort;
         }
   

   The code assigns to $sort the value it already has; which value would the variable have if preg_match() returns 0? I guess it would have the same value, and that mean the IF-statement doesn't do anything.

I forgot to change status.

See digitalnz_api.module. Between a Drupal function and a PHP function with the same purpose, which function would you use?

coder.module doesn't report anything about that because it doesn't have the code to check that; the only way to know if the code is compliant with that point of the coding standards is to read the coding standards.

I keep to forget to change the status.

See the part of the coding standards that refers to the Unicode string functions.

1.   if (empty($key)) {
       drupal_set_message(t('DigitalNZ API needs an API key. Settings are available at <strong class="drupal_link_admin_settings">!link</strong>.', array(
         '!link' => l(DIGITALNZ_SETTINGS_MENU_PATH, DIGITALNZ_SETTINGS_PATH),
       )), 'error');
       return;
     }
   

   See my previous point #5.1.

2.       if ($delta != 'sort') {
         //  return apachesolr_facetcount_form($delta);
         }
   

   What is that code supposed to do?

3.       // Load results.
         /*    <result>
         <category>Images</category>
         <metadata-url>http://api.digitalnz.org/records/v1/1189771</metadata-url>
         <title>Mountain scene</title>
         <date></date>
         <source-url>http://api.digitalnz.org/records/v1/1189771/source</source-url>
         <content-provider>University of Canterbury Library</content-provider>
         <id>1189771</id>
         <description></description>
         <syndication-date>2009-08-12T02:10:49.344Z</syndication-date>
         <display-url>http://132.181.2.68/Data/Collection9/photographs/phg_4933.jpg</display-url>
         <thumbnail-url>http://132.181.2.68/Data/Collection9/photographs/thumbnail/phg_4933.jpg</thumbnail-url>
       </result>*/
   

I should have approved the CVS application before; I am sorry for the delay.

Looking forward to playing with the code. Looks like it may tie in with some stuff I'm doing. Watching the repository :-)