The TweetWall module for Drupal 7.x provides a block to house the functionality provided by TweetWall (http://www.tweetwall.com).

The block configuration features TweetWall-specific options:

  • TweetWall URL
  • Show Header
  • Show Border
  • Border Color
  • Background Color
  • Width
  • Height

Sandbox URL:
https://www.drupal.org/sandbox/cwightrun/2365241

git clone URL:

git clone --branch 7.x-1.x cwightrun@git.drupal.org:sandbox/cwightrun/2365241.git tweetwall

Comments

PA robot’s picture

Status: Active » Needs work

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

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.

cwightrun’s picture

Status: Needs work » Needs review

I'm clear with the validation portion. Please review, thanks!

marcus_johansson’s picture

Status: Needs review » Needs work

It mostly works great, but there are some stuff you need to fix before getting this gets RTBTC.

You need to set the public git clone command not your own, you should change to:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/cwightrun/2365241.git tweetwall

Automated Review

No issues found in pareview.sh

Manual Review

Individual user account
Yes
No duplication
There are many Twitter wall blocks. But not using this specific service. So its no duplication
Master Branch
Yes, follows the guidelines.
Licensing
Yes, follows the guidelines.
3rd party assets/code
Follows the guideline, external code is in an iframe.
README.txt/README.md
No, you should put in something about installation process in the README.txt, no matter how obvious it seems.
Code long/complex enough for review
Maybe - it's over 120 lines long, but not 5 functions. With optimized code it might be under 120 lines. It does show understanding of the hooksystem. Someone else should look into this also to decide.
Secure code
Yes, secure.
Coding style & Drupal API usage
  1. (*) In hook_block_configure you need to use the delta even if you only have one block
  2. (+) Right now you have http://kittens.tweetwall.com as a default value in the form, but not when presented. It doesn't really make sense, you can put it in attributes placeholder if you want an example.
  3. The variables you set in the hook_block_save you should probably delete with the uninstall hook.
  4. Before the block has been configured you should maybe think about not showing it? Some people might enable it without configuring it.
  5. For cleaning up your code on row 141 you should maybe think about using http_build_query/drupal_http_build_query instead since you anyway have an array.
  6. Cleanup code - use $output['border'] = variable_get('tweetwall_show_border') ? 'true' : 'false'; instead of 7 extra rows of code, same for $tweetwall_show_header variable.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

cwightrun’s picture

Thank, Marcus! I'll be addressing these items soon.

PA robot’s picture

Status: Needs work » Closed (won't fix)

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.