[D7] USPS API Integration

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.

The functions are simple and well designed, you are using libraries and that's great. To make things more organized and separated, why you do not create a Class to encapsulate these calls to the API? It would be great. Another point I would leave the usps_api_integration.module only to store hooks and create include files for auxiliary functions.

Standard Review by tool:

FILE: README.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 9 WARNING(S) AFFECTING 9 LINE(S)
--------------------------------------------------------------------------------
  3 | WARNING | Line exceeds 80 characters; contains 95 characters
 13 | WARNING | Line exceeds 80 characters; contains 151 characters
 14 | WARNING | Line exceeds 80 characters; contains 81 characters
 27 | WARNING | Line exceeds 80 characters; contains 128 characters
 30 | WARNING | Line exceeds 80 characters; contains 175 characters
 31 | WARNING | Line exceeds 80 characters; contains 491 characters
 36 | WARNING | Line exceeds 80 characters; contains 97 characters
 50 | WARNING | Line exceeds 80 characters; contains 213 characters
 57 | WARNING | Line exceeds 80 characters; contains 81 characters
--------------------------------------------------------------------------------


FILE: usps_api_integration.module
--------------------------------------------------------------------------------
FOUND 5 ERROR(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
 149 | ERROR | Multi-line array contains a single value; use single-line array
     |       | instead
 202 | ERROR | Multi-line array contains a single value; use single-line array
     |       | instead
 271 | ERROR | Multi-line array contains a single value; use single-line array
     |       | instead
 274 | ERROR | Multi-line array contains a single value; use single-line array
     |       | instead
 394 | ERROR | Multi-line array contains a single value; use single-line array
     |       | instead
--------------------------------------------------------------------------------

I think the feedback in #2 is interesting and useful, but it doesn't seem like any of it rises to the level of issue that would block an application.

I'm not sure about the 5 ERRORs - I've run pareview at http://pareview.sh/pareview/httpgitdrupalorgsandboxmikeroberts2149405git... and do not see those. There's no master branch, so it's not a branch issue. Looking at line 149 to try to find the first error it doesn't seem accurate to the current state of the module.

I've reviewed the module and worked with Mike to fix a few things. I can't find anything more to change about it for now.

Disclosure: Mike and I work together and I reviewed it as a coworker both for our use and for contribution to drupal.org.

manual review:

1. 'Enter your USPS API username.': all user facing text must run through t() for translation.
2. usps_api_integration.test: empty test file?
3. usps_api_integration_install(): why do you need to set these test variables? The whole variable table will be loaded into memory on every single page request, so that only makes your memory footprint worse without any significant production use purpose. You can make use of default values with variable_get() anyway.
4. "variable_get('usps_api_integration_username', ''),": all variables defined by your module need to be removed in hook_uninstall(). Please check all your variables. Looks like all variables mentioned in usps_api_integration.install do not match the ones used in the module file.
5. Could you mention similar modules such as https://drupal.org/project/commerce_usps and https://drupal.org/project/commerce_shipping_usps on your project page and how they are different?

But that are not critical application blockers, 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 greggles as he might ahve time to take a final look at this.

Oops, now really assigning to greggles.

no objections for more than a week, so ...

Thanks for your contribution, mike.roberts!

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.