Description: DrupalTide is a Drupal port of WPTide, which is a WordPress plugin for displaying XTide tidal predictions. This module uses tide prediction data output from XTide in order to allow users of your site to view tide tables and lunar cycles.

Sandbox: http://drupal.org/sandbox/jlea9378/1911416

Drupal 6 Source:
git clone --recursive --branch 6.x-1.x http://git.drupal.org/sandbox/jlea9378/1911416.git drupaltide

Drupal 7 Source:
git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/jlea9378/1911416.git drupaltide

Comments

npscode’s picture

Status: Needs review » Needs work

Hi jlea9378,

Comments after a quick manual review of drupal 7 version :

Your .info files contains release version. It automatically gets added by the packaging script on drupal.org (the version/info/timestamp info). These lines should be removed from your .info file. Have a look at an established contrib module's info file, you'll see which lines automatically get added.

I also ran your module through PAReview, which lists all the code style issues, and a few other ones, see http://ventral.org/pareview/httpgitdrupalorgsandboxjlea93781911416git for the report. You can re-run the review after you've made changes, to verify your progress.

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732

Try to use standard way of drupal 7 for selecting data from tables. refer : api.drupal.org/api/drupal/includes!database!database.inc/function/db_select/7. db_query() (at line 61 in drupaltide.module) will work but try to avoid this in drupal 7.

Thanks..

dDoak’s picture

Hi,

A quick manual review :

- in drupaltide_block_info if (TRUE) is not necessary code.
- in drupaltide_block_view you should externalize HTML code in a template. Create a theme entry via hook_theme
- I d'ont understand why you alter your own schema during hook_install process. Why don't declare the foreign key in hook_schema : http://drupal.org/node/146939

Thx

he0x410’s picture

Hi,

From manual review, comments are:

drupaltide.module

  1. Use file header comment in format:
    /**
    * @file
    * file description goes here
    */
  2. In function drupaltide_block_info@8, use code format as example on http://api.drupal.org/api/drupal/modules%21block%21block.api.php/functio...
  3. Why do you use if (TRUE) ? It needs to be removed
  4. In function drupaltide_block_view@20:
    • Use code format as example on http://api.drupal.org/api/drupal/modules%21block%21block.api.php/functio...
    • Unused variables needs to be removed: $block_content@29, $tide_table_name@31, $tist_table_name@32, $datearray@57
    • Arrays $dayofw and $namonth, can be initialized like $dayofw = array('Sun', 'Mon', ...); (Remember not exceed 80 chars per line)
    • Inline comments should be with double slashes //, not with /* */
    • Use theme function for HTML output
    • Use t function for user-end strings for translation compatibility
    • Avoid using a lot of double quotes for strings, where you're not going to insert php variables.
jlea9378’s picture

Thanks for the feedback. I will try to make the corrections and comment back here with questions or to let you know that I've made the changes. It might be a week or so before I can get it done.

PA robot’s picture

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

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

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