[D6] uc_correios_webservices

welcome,

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page. Also create a README.txt that follows the guidelines for in-project documentation.

An automated review of your project has found looks good, though there are some small errors left you can fix (http://ventral.org/pareview/httpgitdrupalorgsandboxrodrigoprior1651150git)

We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

regards

Hi Rodrigo,

Some comments from a manual review:

• Your hook_perm defines a permission called 'administer correios quotes'. You don't use this permission in your code.
• In hook_menu you use the permission 'configure quotes'. I suppose this permission comes from another module?
• Small typo on line 168 and 194 of uc_correios_webservices.module: "costumers" should be "customers".
• On line 631 I would recommend to build the entire error message using the t() function instead of using string concatenation. It's not really a security risk if you don't do it since you already do a check_plain before, but it is the standard way of building strings in Drupal. Also when doing the concatenation inside t(), you give translaters more flexibility on how they want to have the error displayed. I would recommend something like
  t('error: !error_code - !error_msg', array('!error_code' => $delivery_error_code, '!error_msg' => $delivery_error_msg))
  Also note that you can build the error message using the watchdog function. The syntax would become:
  watchdog('correios', 'error: !error_code - !error_msg', array('!error_code' => $delivery_error_code, '!error_msg' => $delivery_error_msg), WATCHDOG_ERROR);
• On line 637 you check whether $delivery_user_error_msg equals the integer zero. However the function uc_correios_webservices_user_error_messages() returns strings and not an integer.

Hi,

This module is looking pretty good to me.

A couple of minor issues from a manual review:

You may want to consider adding a leading underscore to functions which are private to this module.

There seem to be a few strings which are in Spanish (I think?) where as most of them are in English. For example on line 186: '#description' => t('ADICIONAR TEXTO DE AJUDA'),

Overall though I think that it looks pretty well written - these are just minor issues.

hey guys, thanks for participation, but please remember:
1. review oldest applications first, here are people waiting for a review for months!
2. After you've done a manual review either change to "needs work" if you found major issues, or RTBC if you found no major issues
thanks

Duplicate of #1663310: [D6] uc_correios_webservices. Please open only one application in one issue.