gionnibgud [gionnibgud]

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

As per requirements, the motivation message should be expanded to contain more features of the proposed project. For themes, it should include also a screenshot of the theme, and (when possible) a link to a working demo site; for modules, it should include also a comparison with the existing solutions.

• The points reported in this review are not in order of 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.

1. License files cannot be committed in Drupal.org repository. Projects committed in Drupal.org repository have the same license used by Drupal.
2. Hook implementation comments should be like the following one:

   /**
    * Implements hook_menu().
    */
   

   As reported in Documenting hook implementations:

   If the implementation of a hook is rather standard and does not require more explanation than the hook reference provides, a shorthand documentation form may be used in place of the full function documentation block described above:

   /**
    * Implements hook_help().
    */
   function blog_help($section) {
     // ...
   }
   

   This generates a link to the hook reference, reminds the developer that this is a hook implementation, and avoids having to document parameters and return values that are the same for every implementation of the hook. Optionally, you can add more information in a separate paragraph to describe the particular quirks of your hook implementation.

   In the case of hooks that have variables in the names, such as hook_form_FORM_ID_alter(), a slightly expanded syntax should be used:

   /**
    * Implements hook_form_FORM_ID_alter() for node_type_form().
    */
   function mymodule_form_node_type_form_alter(&$form, &$form_state) {
     // ...
   }
   

   This generates a link to the hook reference, as well as to the particular form that is being altered. Again, optionally you can add more information in a separate paragraph to describe the particular quirks of your hook implementation.

3.       return t('1. Create a Vocabulary
   2. Add some terms to your newly created vocabulary
   3. Create or edit a webform
   4. Add a select component
   5. Use the pre-built option list select to select a Vocabulary
   6. Save the component
   
   You should now have a select component populated with all the terms from the Vocabulary you\'ve chosen.');
   
   

   Avoid to escape a string delimiter inside a string, especially if the string is passed to t(). Why isn't the string using HTML tags?

Hi,

As far as I see functionally it looks OK.

All text needs translating with t() which you have done, but all HTML output (as I understand it) needs to be themeable (so that someone can override the layout, rather like with t() people can override the translation). See:
http://drupal.org/node/306
http://drupal.org/node/165706

Specifically, for the following code, I think you should use http://api.drupal.org/api/drupal/includes--theme.inc/function/theme_item... :

function webform_taxonomy_select_help($path, $arg) {
  switch ($path) {
    case 'admin/help#webform_taxonomy_select':
      return t('<ol>
        <li>Create a Vocabulary</li>
        <li>Add some terms to your newly created vocabulary</li>
        <li>Create or edit a webform</li>
        <li>Add a select component</li>
        <li>Use the pre-built option list select to select a Vocabulary</li>
        <li>Save the component</li>
        </ol>
        <p>You should now have a select component populated with all the terms from the Vocabulary you have chosen.</p>');
  }
}

I think the convention for README naming is README.txt rather than README.TXT

Hi,

I don't have CVS rights so I can't help much further than the initial review. You will need to wait for someone else to take this further.

Good luck!

Hi. Please read all the following and the links provided as this is very important information about your CVS Application:

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for your application. Please read the following documentation on how this affects and benefits you and the application process:
Migrating from CVS Applications to (Git) Full Project Applications

• The status of this application will be put to "postponed" and by following the instructions in the above link, you will be able to reopen it.
• Or if your application has been "needs work" for more than 5 weeks, your application will be marked as "closed (won't fix)". You can still reopen it, by reading the instructions above.

As per previous comment, I am setting this issue to won't fix.

Since new users can now create full projects, applications have a different purpose and they are handled on a different issue queue. See Apply for permission to opt into security advisory coverage for more information.