[D7] Commerce track order

Thanks Anup,

I will review it

Unassigning this issue as it is for community review

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 Anup,
i reviewed your module please fix below issues:
1) Add hook_help(not a blocking issue)
2) use t function in queue_listing_order.inc (not a blocking issue)
3) track_styles.css should be inside css folder (not a blocking issue)
4) Please add more informations in readme.txt and project page(not a blocking issue)
5) Remove php tags for file doc comment, they are not required.

<?php

/**
 * @file
 * Template file for the grid skin.
 */
?>

@vipul.patil7888: Thanks for your review! Is this now RTBC or are there application blockers left and this should be "needs work"?

Hi Klausi,
As per my review, i have not found any blocking issue, though minor issues related to drupal standards needs to be fixed.
Thanks

Automated Review returned quite alot of issues and, while many come from miss-indented lines, there are still some fairly big ones:

• "all functions should be prefixed with your module/theme name to avoid name clashes", found in some functions. Please try to use commerce_track_order_ prefix for all your module functions to avoid collisions
• some unused variables here and there (no, they are not false positives)

There are some more, but the pareview.sh page displays them better.

Given the fact that there are so many issues, I will not do a manual review just yet. Can you please fix the issues reported by the automated review ? Thanks.

Hey there anup,

The Automated Review returned only 4 missindented lines (4 spaces instead of 2) on commerce_track_order.admin.inc - lines 24-27. This should be fixed. Otherwise, looks good.

I took a more in-depth look at your project and here's my manual review:

Coding style & Drupal API usage
(*) You have a call to system_settings_form() in commerce_track_order_configuration() that creates 2 variables that the project uses, but those variables are not deleted in hook_uninstall()
(*) In csv_upload.inc, at line 26, you have a partially translated string (' To download CSV Template' is not translated)
(+) In your .module file, at line 272, you have a module_invoke_all call, but that hook is not implemented in .api.php (the file does not exist); every hook you declare shoul have a snippet in .api.php and good documentation for other users to be able to implement it

Recommendations (minor, but could improve the quality of your code):

• You may want to move all admin pages/forms into .admin.inc like you did for a few of them
• In .info file you have defined alot of dependencies; for readability reasons, you may reduce them to those that your module actually depend on; if you declare a module as a dependency, your project will depend on every dependency that module depends on
• In .queue.inc, for commerce_track_order_retrieve_queue(), you have a docblock that missed the @return tag
• in .module, for commerce_track_order_configuration(): in docblock, you have @return form that should be @return array and a typo in @return description (Retuens)
• in .module, at line 181, you declare a variable $defult_value; it should read $default_value (declaration and every use) for better readability
• in .csv_upload.inc, at line 5, another typo (funcationality)

@web247: wow, nice review! I think most of the points are pretty minor and should not hold up this application, anything else that you found or should this be RTBC instead?

Hi Anup,

i reviewed your module.
Please Make sure you always use t() to use Strings. in Your Template files, Strings are used without t().

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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