[D7] Twitter Images

Hi Ashley George,

I like the module idea.

For a first test I've passed it through pareview.sh and got a number of errors returned.
See here - http://pareview.sh/pareview/httpgitdrupalorgsandboxashleygeorge2349485git

Many of them seem quite easy; have a look at consistent function names inline with twitter_images_XXXXX and passing readable text through the t() function.

Best of luck with the review process!

Rick

I've changed the issue inline with these standards too - https://www.drupal.org/node/1011698

Hi Ashley George,

Thanks for the contribution!

I had issues installing but they seem to be known issues with the twitter modules dependency on the oauth_common module.
Installation working fine if OAuth already present before installation.

I'll take a closer look next week.

Manual code review:

General
Variable naming conventions:
Please follow the naming convention for your modules variables to reduce the chance of clashing with variables created by other modules:
mymodule_variable
https://www.drupal.org/coding-standards#naming
It looks like you've been using this for some but not all variable names. I also spotted a spelling mistake in the variable name: twitter_imaegs_cron_next_execution on line 119 of the .module file that will be causing a bug in the output sent to watchdog

.info file
Style issue:
package = Custom
it would be nice if this was the same as the twitter module's package to keep them all together.

.install file:
Please remove all the variables you use in the module in the hook_uninstall

.admin.inc
Style / Documentation issue:
I'd suggest hiding the "Maximum cron frequency" setting dependant on the state of the "Automatically look for new Tweets" checkbox. Also, it might be worth rephrasing the descriptions of the two cron items, the difference in language used (no mention of cron in the checkbox description) could lead to confusion.

.form.inc
twitter_images_form
This functions name means it will get picked up as implementing hook_form() which doesn't appear to be what you want.
https://api.drupal.org/api/drupal/modules!node!node.api.php/function/hoo...

would suggest a more verbose name e.g. twitter_images_configuration_form.

drupal_set_title($title);
You shouldn't need this as you are setting the title in your hook_menu implementation

Hi Ashley,

This module has been ideal for my work, so thanks!

There is an issue with the date format for the tweet date. It shows as a long number than a formatted date in the view.

Kirsty

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.

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.

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,

Please see my observations below:

1) Please add git clone url in project page. Refer Project page template for more details.

2) Add space between if and open parenthesis in line no 57.

if(!isset($record->image_url) || empty($record->image_url) ||
          !isset($record->tweet_url) || empty($record->tweet_url)){
        continue;
      }

3) Add more detailed and structured details in README.txt file.
It would be clear about the module installation, other dependant modules. If you are using libraries that also please mention in REAME.txt file. Currently it looks like unstructured.

twitter_images.form.inc

4) Pass variable as arguments instead of directly concatinate with string in t() function in line no. 145.

$form['no_data'] = array(
      '#markup' => t('There are currently no !title. Configure how Tweets are imported on the !link.', array(
        '!link' => l(t('config page'), 'admin/config/services/twitter_images/settings'),
      ), array('!title' => $title)),
    );

5) Fix all issues showing in PAReview.
http://pareview.sh/pareview/httpgitdrupalorgsandboxashleygeorge2349485git
and add this link in issue page.

Nice observations ARUN AK..
Setting the status to "Needs Work", so that future reviewers do not have same complaints/issues.
Ashley George, please fix the issues and change the status to "Needs Review".

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.