[D7] Webform Calculation Components

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxanthony_goode2272403git

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

Your branch name should be changed from 'master' to something like '7.x-1.x' to follow proper dev guidelines.

In the js files and the .module file, I'd removed the commented out code for the console.log() calls. Commented out debug code shouldn't be released.

I'd removed the .install file since it is basically empty except for the comments.

Would it be possible to refactor webform_calculation_components_form_alter() to reduce the amount of code duplication? It seems like there is a lot of boilerplate between the number and calc types, which can lead to maintenance problems down the line.

You have changed your branch from master to 7.x-1.x but your git clone path is still wrong and you have provided git path which is only for you because it will ask for password.
So please change it to:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/anthony_goode/2272403.git webform_calculation_components

The wrong git link isn't reason enough to warrant going back to **Needs Work**, especially if it is the only thing being reported (and anyone can edit the issue summaries to fix this). I changed the git link.

Were there any problems with the module or code itself? Setting back to **Needs Review**

Thanks your for your reviews. When finishing your review comment also set the issue status either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no major flaws).

Review of the 7.x-1.x branch:

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

  
  FILE: /home/klausi/pareview_temp/js/webform_calc_number_component.js
  --------------------------------------------------------------------------------
  FOUND 1 ERROR AFFECTING 1 LINE
  --------------------------------------------------------------------------------
   1 | ERROR | Missing file doc comment
  --------------------------------------------------------------------------------
  
  
  FILE: /home/klausi/pareview_temp/js/webform_calc_hidden_component.js
  --------------------------------------------------------------------------------
  FOUND 1 ERROR AFFECTING 1 LINE
  --------------------------------------------------------------------------------
   1 | ERROR | Missing file doc comment
  --------------------------------------------------------------------------------
  

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. https://www.drupal.org/project/webform_calculator - please describe the difference to the existing project on your project page.
2. The Git commits are not connected to your user account. You need to specify an email address. See https://www.drupal.org/node/1022156 and https://www.drupal.org/node/1051722
3. There are a couple of commented out dpm() calls in your code.
4. "@to-do" should be just "@todo", see https://www.drupal.org/coding-standards/docs#todo
5. I'm not that familiar with webform logic, but it seems you are applying the XSS sanitization functions in the right places.

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 heddn as he might have time to take a final look at this.

Automated review:

     webform_calc_number_component.js    Duplicate declaration at line 22, 26, 30, 34, 38, 42
     webform_calc_number_component.js    Variable result_field implicitly declared at line 21, 29, 32, 69
webform_calculation_components/components/wb_calc_number.inc  Variable 'decimals' might have not been defined (at line 272)
components/wb_calc_number.inc  Variable 'matches' might have not been defined (at line 272)

Manual review:

• Don't concatenate into t(). Use string replacement arguments. Same applies in _webform_edit_wb_calc_number().
  

  function _webform_edit_wb_calc_hidden($component) {
    ...
      '#description' => t('The default value of the field.') . ' ' . theme('webform_token_help'),

• Don't use html in t(); it makes translation more difficult. Instead use '%' replacement variables for emphasis.
  

  function _webform_edit_wb_calc_hidden($component) {
  ...
  $form['display']['calculation_hidden_type'] = array(
  
      '#description' => t('Both types of webform hidden calculation fields are not shown to end-users. Using a <em>Secure value</em> allows the use of <em>all tokens</em>, even for anonymous users.'),

• Don't concatenate t() strings together. Just make a longer string. I see this in a couple places.
  

  function _webform_edit_wb_calc_number($component) {
      '#description' => t('Width of the webform calculation number field.') . ' ' . t('Leaving blank will use the default size.'),
      '#size' => 5,
      '#maxlength' => 10,
      '#weight' => 0,
      '#parents' => array('extra', 'width'),

None of these are blocking issues though, so...

Thanks for your contribution, Anthony!
I updated your account so you can 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 stay 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.

Unfortunately I can't fix that for you. Please open an issue to the d.o. infrastructure team. An issue with a similar problem to get a feel for the process: https://www.drupal.org/node/1421354