chrislynch42 [chrislynch42]

Hello, and thanks for applying for a CVS account. I am adding the review tags, and some volunteers will review your code, pointing out what needs to be changed.

As per http://drupal.org/cvs-application/requirements, the motivation message should be expanded to contain more details about the features of the proposed module/theme; for modules it should include also a comparison with the existing solutions, while for themes a screenshot is also required.

Thanks for the reply.

Please look at Drupal Coding standards, there are issues with indenting, etc. Drupal Coding Standards are really important for other developers so they can help you with the module.

• The points reported in this review are not in order or importance / relevance.
• Most of the times I report the code that present an issue. In such cases, the same error can be present in other parts of the code; the fact I don't report the same issue more than once doesn't mean the same issue is not present in different places.
• Not all the reported points are application blockers; some of the points I report are simple suggestions to who applies for a CVS account. For a list of what is considered a blocker for the application approval, see CVS applications review, what to expect. Keep in mind the list is still under construction, and can be changed to adapt it to what has been found out during code review, or to make the list clearer to who applies for a CVS account.

•     db_query("delete from {csl_bt_author} WHERE nid = %d ", $node->nid);
      db_query("delete from {csl_bt_author_book} WHERE nid = %d ", $node->nid);
      db_query("delete from {node} where nid=%d ",$node->nid);
      db_query("delete from {node_revisions} where nid=%d ",$node->nid);
  
  

  Reserved SQL words are written in upper case.

• See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how the code should be formatted; how Drupal variables, global variables, constants, and functions defined from the module should be named; how constants should be written.

•   $form['#redirect'] = $thepath;
    $form_state['redirect'] = $thepath;
  
  

  A submission handler should just use the last code line.

• Remove any debug code.
  Remove also any not necessary files, like the ones used from the IDE you are using, and the screenshots.

• package = Seafarer
  version = VERSION
  

  The version line needs to be removed from the .info file.
  What does Seafarer mean?

1. Menu descriptions and titles, schema descriptions are not passed to t().
2. Submodules that are part of the same project should have the name prefixed by the project name.

3. function milan_perm() {
     return array('create milan entries',
                           'delete own milan entries',
                           'delete any milan entry',
                           'edit own milan entries',
                           'edit any milan entry',
                           'view any milan entry',
                           'administer milan series',
                           'administer milan genre',
                           'administer milan world',
                           'administer milan');
     
   }
   

   The first 6 permissions are already defined from the node module.
   What is the meaning of milan, in these cases?

4. //////////////////publisher select box ajax render
   

   Function comments don't follow our standards.

5. Remove any debug code.

6. package = Core - optional
   version = VERSION
   core = 6.x
   

   The module is not part of Drupal core.

7.          $thepath .= "/" . strtoupper(substr($form_state['values']["title"],0,1));
   
   

   Why isn't the code using drupal_ucfirst()?
   As per coding standards, the code should use the Drupal Unicode functions.

8.     //Create list of letters across top
     $result = db_query("SELECT DISTINCT substr(replace(upper(n.title),'THE',''),1,1) theLetter FROM {node} n WHERE n.type = 'publisher' AND n.status = 1 ORDER BY theLetter");
     $output .= l('[ALL]','milan/publisher');
   
   

   Strings used in the user interface should be translated.

I am closing this issue due to lack of replies.

Please re-apply for a CVS account; this application has been left without replies, and it it has been already declined. Changing the status of this report doesn't change the status of the CVS application.