Activity Stream For Google+

Hi,

I performed an automated review on your code and quite some code formatting issues popped up.
Can you please fix these? See http://ventral.org/pareview/httpgitdrupalorgsandboxsgtsaughter1416090git

Also your activity_stream_for_google_.info has to contain more content. The fields description and core are required elements in the file. See http://drupal.org/node/231036

And last but not least, there is the problem that your submitted code is too short.
This url How much code do we need to approve a user? proposes simple rules that can be used to deny the "create full projects" permission (everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed).

Hi,

Here some information about how to move to a different branch: Moving from a master to a major version branch.

I performed my review based on the code which was in the Drupal Git repository. That's where I saw the issue with your .info file.
Can you please move your code from Github to the Drupal Git?

Thanks.

Hi,

Did manual review of your code. Few small issues here. You should implement following things in your module for drupal standards.
1. Always put comment above your function to tell what it does.
2. if you are using a hook you should comment "Implements hook_foo().". This is the standard now. You have written hook for user settings form, which is incorrect.
3. Also leave an extra line at end of your module. Always end your module or inc file with an new line.
You also havent mentioned core in your info file, i believe it should always be there.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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.

Please find automated review results below and fix the same.

http://pareview.sh/pareview/httpgitdrupalorgsandboxsgtsaughter1416090

FILE: ...ar/www/drupal-7-pareview/pareview_temp/activitystream_googleplus.module
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AND 2 WARNING(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
12 | ERROR | Expected 1 space between asterisk and tag; 0 found
29 | ERROR | Expected 1 space between asterisk and tag; 0 found
40 | ERROR | Expected 1 space before ":"; 4 found
47 | WARNING | Format should be "* Implements hook_foo().", "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar().", or "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar.tpl.php.".
56 | WARNING | Format should be "* Implements hook_foo().", "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar().", or "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar.tpl.php.".
--------------------------------------------------------------------------------

Cheers,
Anil Sagar

minor coding standard errors are not application blockers, please do a real manual review.

Hi,

1. activitystream_googleplus.module line 65: Writing variables inside the double quotes is not a better practice. Use concatenation to split the strings and variables.

"<span>$name "
"<span class=\"activitystream-created\">$date</span></span>"

2. i don't think you need a separate theme function for getting the googleplus icon which is used only once inside theme_activitystream_googleplus_item function.

Thanks

@silambarasan: Having variables in "" double quotes is fine with the Drupal coding standards, where did you get that information that it is not a good practice?

@silambarasan Having variables in double quotes is generally fine.

AUTOMATED CODE REVIEW
=====================

There are several code errors and warnings to take care of as noted below:
http://pareview.sh/pareview/httpgitdrupalorgsandboxsgtsaughter1416090

FILE: ...ar/www/drupal-7-pareview/pareview_temp/activitystream_googleplus.module
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AND 2 WARNING(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
12 | ERROR | Expected 1 space between asterisk and tag; 0 found
29 | ERROR | Expected 1 space between asterisk and tag; 0 found
40 | ERROR | Expected 1 space before ":"; 4 found
47 | WARNING | Format should be "* Implements hook_foo().", "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar().", or "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar.tpl.php.".
56 | WARNING | Format should be "* Implements hook_foo().", "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar().", or "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar.tpl.php.".
--------------------------------------------------------------------------------
DrupalPractice has found some issues with your code, but could be false positives.
@silambarasan Having variables in double quotes is generally fine.

AUTOMATED CODE REVIEW
=====================

There are several code errors and warnings to take care of as noted below:
http://pareview.sh/pareview/httpgitdrupalorgsandboxsgtsaughter1416090

FILE: ...ar/www/drupal-7-pareview/pareview_temp/activitystream_googleplus.module
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AND 2 WARNING(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
12 | ERROR | Expected 1 space between asterisk and tag; 0 found
29 | ERROR | Expected 1 space between asterisk and tag; 0 found
40 | ERROR | Expected 1 space before ":"; 4 found
47 | WARNING | Format should be "* Implements hook_foo().", "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar().", or "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar.tpl.php.".
56 | WARNING | Format should be "* Implements hook_foo().", "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar().", or "* Implements
| | hook_foo_BAR_ID_bar() for xyz_bar.tpl.php.".
--------------------------------------------------------------------------------
DrupalPractice has found some issues with your code, but could be false positives.

FILE: ...ar/www/drupal-7-pareview/pareview_temp/activitystream_googleplus.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
52 | WARNING | Do not call theme functions directly, use theme('image', ...)
| | instead
--------------------------------------------------------------------------------

1. BASIC APPLICATION CHECKS
===========================
Okay

2. BASIC REPOSITORY CHECKS
==========================
Okay

3. SECURITY REVIEW
==================
For function activitystream_googleplus_streamapi($user), the password should
probably be salted or otherwise protected. At a glance looks like the data is
passed as plain text. However, this is an issue for the activitystream module.

4. LICENSING CHECKS
===================
Not sure if the google icon conforms to Google brand standards and licensing.
https://developers.google.com/+/branding-guidelines

5. DOCUMENTATION CHECKS
=======================
Sentences in README.txt file are missing periods. Try to use complete
sentences.

6. CODING STANDARDS AND STYLE
=============================
1. When implementing a hook, use a short summary of the form "Implements hook_menu().". Omit the parameter and return value documentation in this case.
https://drupal.org/coding-standards/docs last bullet in section: Drupal API documentation standards (general).

2. Concatenated strings using (.) that break to new lines is not very readable.
See https://drupal.org/coding-standards#linelength for handling long lines.

"In general, all lines of code should not be longer than 80 chars.
Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 chars.
Control structure conditions may exceed 80 chars, if they are simple to read and understand: "

Line 39 is an example of a line that should be full length. The t() calls can also be full lines. Drupal Core is full of super long t() usages.

3. For Line 65, build an array and simply return the array as a variable.
Drupal has render functions that will print arrays nicely in the theme layer.
For official documentation on this practice see section: "Creating Content As Render Array" of https://drupal.org/node/930760

4. For line 42 use l() instead of an anchor tag and insert l() inside of t() using replacements.

An example from https://api.drupal.org/api/drupal/includes!bootstrap.inc/function/t/7

<?php
    $message = t("If you don't want to receive such e-mails, you can change your settings at !url.", array('!url' => l(t('My account'), "user/$account->uid")));
?>

7. API AND BEST PRACTICES REVIEW
================================

1. Remove (project = "activitystream") from the .info file. Drupal will add the
project and version information.

2. For the form $form['activitystream_googleplus'] on line 32, I recommend
writing a validation function for the field. You should probably check to make
sure it is a valid google URI and other checks.

For more info https://api.drupal.org/api/drupal/developer!topics!forms_api_reference.h...

FILE: ...ar/www/drupal-7-pareview/pareview_temp/activitystream_googleplus.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
52 | WARNING | Do not call theme functions directly, use theme('image', ...)
| | instead
--------------------------------------------------------------------------------

1. BASIC APPLICATION CHECKS
===========================
Okay

2. BASIC REPOSITORY CHECKS
==========================
Okay

3. SECURITY REVIEW
==================
For function activitystream_googleplus_streamapi($user), the password should
probably be salted or otherwise protected. At a glance looks like the data is
passed as plain text. However, this is an issue for the activitystream module.

4. LICENSING CHECKS
===================
Not sure if the google icon conforms to Google brand standards and licensing.
https://developers.google.com/+/branding-guidelines

5. DOCUMENTATION CHECKS
=======================
Sentences in README.txt file are missing periods. Try to use complete
sentences.

6. CODING STANDARDS AND STYLE
=============================
1. When implementing a hook, use a short summary of the form "Implements hook_menu().". Omit the parameter and return value documentation in this case.
https://drupal.org/coding-standards/docs last bullet in section: Drupal API documentation standards (general).

2. Concatenated strings using (.) that break to new lines is not very readable.
See https://drupal.org/coding-standards#linelength for handling long lines.

"In general, all lines of code should not be longer than 80 chars.
Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 chars.
Control structure conditions may exceed 80 chars, if they are simple to read and understand: "

Line 39 is an example of a line that should be full length. The t() calls can also be full lines. Drupal Core is full of super long t() usages.

3. For Line 65, no need to return a mess like this. Build an array and simply return the array as a variable.
Drupal has render functions that will print arrays nicely in the theme layer.
For official documentation on this practice see section: "Creating Content As Render Array" of https://drupal.org/node/930760

4. For line 42 use l() instead of an anchor tag and insert l() inside of t() using replacements.

An example from https://api.drupal.org/api/drupal/includes!bootstrap.inc/function/t/7

<?php
    $message = t("If you don't want to receive such e-mails, you can change your settings at !url.", array('!url' => l(t('My account'), "user/$account->uid")));
?>

7. API AND BEST PRACTICES REVIEW
================================

1. Remove (project = "activitystream") from the .info file. Drupal will add the
project and version information.

2. For the form $form['activitystream_googleplus'] on line 32, I recommend
writing a validation function for the field. You should probably check to make
sure it is a valid Google URI etc.

For more info https://api.drupal.org/api/drupal/developer!topics!forms_api_reference.h...

Setting to status to Needs Work.

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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