[D7] Betaout

Git clone command for the sandbox is missing in the issue summary, please add it.

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.

Updating issue summary as this is not "Fixed" yet per se.

When your application ready to be reviewed, please change status to "Needs Review". The "Fixed" status is for when the application is approved by admins.

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

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

Manual review

Fix your git clone. It should be git clone --branch 7.x-1.x http://git.drupal.org/sandbox/betaout/2473053.git betaout
betaout.module: remove empty hook_init()
betaout.admin.inc: Persistent variables should be named in all lowercase (see https://www.drupal.org/coding-standards#naming)
betaout.api.php: empty file
betaout_defaults.module: Use module_load_include to load include files
betaout_defaults.module: function betaout_defaults_betaout_defaults_alter() implements hook that does not exist
betaout_defaults.node.inc, betaout_defaults.user.inc & betaout_defaults.comment.inc: Implement multiple empty hooks with no functionality

Hi, it looks like this is getting very close!

There is one very minor problem still being caught by PAreview.sh:

Review of the 7.x-1.x branch (commit 8488c86):

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

FILE: /var/www/drupal-7-pareview/pareview_temp/betaout.module
---------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------
299 | ERROR | [x] Array closing indentation error, expected 4 spaces but
| | found 5
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------

Time: 314ms; Memory: 10Mb

Please fix this and then mark the status "Needs Review" again. Good luck!

Minor coding standard issues are surely not application blockers, anything else that you found or should this be RTBC instead?

Please don't assign the project application issue to yourself if you want other people to review it. Please see this page about assigning ownership to issues.

Before adding tags read the issue tag guidelines. Do NOT use tags for adding random keywords. I've removed the random word you added.

Do not include .gitignore in the repo you push (make it ignore itself).

Thanks for the contribution it looks like an interesting one, but there does looks like some clean up and formatting needs to be done, I've added my notes below, they may or may not be important.

Automated Review

Fine apart from a very few small formatting issues returned by PAReview but nothing major.
http://pareview.sh/pareview/httpgitdrupalorgsandboxbetaout2473053git-7x-...

Manual Review

Individual user account

[No] Does not follow the guidelines for individual user accounts.

No duplication

[Yes] Does not cause module duplication and/or fragmentation.

Master Branch

[Yes] Follows the guidelines for master branch.

Licensing

[Yes] Follows the licensing requirements.

3rd party assets/code

[Yes] Follows the guidelines for 3rd party assets/code.

README.txt/README.md

[Yes] Follows the guidelines for in-project documentation and/or the README Template. However you might want to take a look at the README template for improvements: https://www.drupal.org/node/2181737

Code long/complex enough for review

[Yes] Follows the guidelines for project length and complexity.

Secure code

[Yes] Meets the security requirements.

Coding style & Drupal API usage

* line 364; betaout_user_logout() doesn't do anything.
* line 478; betaout_make_request() ignores the exception, perhaps some logging should go here. There are a few other places where exceptions are not being handled, in the betaout_defaults.ubercart.inc file for instance.
* betaout.admin.inc; variable betaout_track_anonymous doesn't seem to be used anywhere.
* line 24; betaout_defaults_node_view in betaout_defaults.node.inc assumes that the uc_product_image field exists which might cause an error if it doesn't? If Ubercart is required it should be listed as a requirement in the betaout.info file.
* _betaout_addtocart(), _betaout_updatecart(), _betaout_product_view(), _betaout_removefromcart() and _betaout_purchased() functions seem identical apart from the action, perhaps a single function in which the action is provided as a parameter would reduce duplicate code.

betaout_order() line 266 in betaout_defaults.ubercart.inc doesn't do anything.

$totaldiscount line 219 in betaout_defaults.ubercart.inc isn't being used and is re-declared after the unhanded exception

A small formatting nitpick, arrays seem to be defined empty with the contents place beneath it line by line, it would look nicer if the array was formed with one assignment i.e.:

 $product = array(array(
   'sku'               => $node->model,
   'productId'     => $node->vid,
   'productTitle' => $node-title,
 ));

I've noticed in the betaout_page_build() function it adds the $user_email and $user_id to the script even if they're empty after checking the uid. Not sure if this is an issue or not.

Some of the comments are missing parameters and return values and have a few typos.

In the betaout_get_defaults() function you pull in the email from the cookie if it's present, I'm not sure if this could present a security issue in anyway, probably not but just a small observation.

@splendidles: I think you forgot to change the status. I this now RTBC after your review or are the issues you pointed out application blockers and this should be "needs work"?

Individual user account

[Yes] Follows the guidelines for individual user accounts.

?? Sure looks like a company account to me - i.e. NON-individual account ....

That's a blocker:

PAReview: Individual user account

It seems you are using a non-individual account.
All user accounts are for individuals. Accounts created for more than one user or those using anonymous mail services will be blocked when discovered (see Get a Drupal.org account).
Please note that organization accounts cannot be approved for git commit access. See https://drupal.org/node/1966218 and https://drupal.org/node/1863498 for details on what is/isn't allowed. Please update your user profile so that we don't have to assume that this is a group account.

Also note that 7.x-1.x-rc1 is not a valid branch name for for use in the Drupal git repository. See https://www.drupal.org/node/1015226

Apologies; I've not done many reviews and still getting the hang of the procedure. Needs work I would say, and TR is right there, my mistake.

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.