[D7] Commerce Checkout Information

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

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.

HI,

What about localization?
Also better if you add "_" (underscore) after variable name. For example here
variable_set(COMMERCE_CHECKOUT_INFORMATION_VARIABLE_PREFIX . $form['checkout_pane']['#value']['pane_id'], $form_state['values']['checkout_information_text']);

It will look much better if somebody looks on the name of the variable.

I see. Ok.

Have you check this http://pareview.sh/pareview/httpgitdrupalorgsandboxlslinnet2186901
There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732 .

You can delete a branch if you have merged in everything from that branch. So if you are working in a development branch and you have merged in all the changes from master if any exist, you can delete the master branch. You will not loose anything because you have already merged the changes in.

Please add all your review bonus reviews to the issue summary, so that we can track them.

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
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/README.md
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AND 8 WARNING(S) AFFECTING 9 LINE(S)
  --------------------------------------------------------------------------------
    7 | WARNING | Line exceeds 80 characters; contains 108 characters
    9 | WARNING | Line exceeds 80 characters; contains 126 characters
   13 | WARNING | Line exceeds 80 characters; contains 100 characters
   16 | WARNING | Line exceeds 80 characters; contains 88 characters
   20 | WARNING | Line exceeds 80 characters; contains 303 characters
   24 | WARNING | Line exceeds 80 characters; contains 124 characters
   30 | WARNING | Line exceeds 80 characters; contains 180 characters
   31 | WARNING | Line exceeds 80 characters; contains 270 characters
   32 | ERROR   | Files must end in a single new line character
  --------------------------------------------------------------------------------
  

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. commerce_checkout_information_form_alter(): instead of mixing two different form alter purposes I would recommend to implement hook_form_FORM_ID_alter() for commerce_checkout_pane_settings_form instead to keep this clearly separated.
2. commerce_checkout_information_form_alter(): don't call theme() here, just add to the nested render array. Drupal will render everything later for you. See https://drupal.org/node/930760
3. commerce_checkout_information.js: Empty file comment? And the JS does not do anything (yet)? That should be @todo then?

But otherwise looks RTBC to me.

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

Thanks for your contribution, Lars.

No further objections from me (only a small hint).
I suggest integrating your module with Variable so you don't need custom code for translation of your variables. If you "register" your variables using hook_variable_info() you can remove all the custom language stuff from your module. The translation is done by i18n and Variable itself so you don't need check the current language on load/save.

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.