Drupal.org
Drupal.org CVS applications » Issues

ForexFeed [forexfeed]

Posted by ForexFeed on August 1, 2009 at 5:55am
Project:Drupal.org CVS applications
Component:Miscellaneous
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

CVS edit link for ForexFeed

Hello,

We've created a new module and would like to contribute it to the community.

This module displays real-time currency quotes (aka FOREX) in configurable block(s), it is designed to operate in conjunction with the data services of forexfeed (dot) net.

Thank you.

Login or register to post comments

Comments

#1

Posted by ForexFeed on August 1, 2009 at 5:57am
AttachmentSize
forex_feed.tar_.gz 69.17 KB

#2

Posted by ForexFeed on August 1, 2009 at 6:02am
Status:postponed (maintainer needs more info)» needs review

#3

Posted by kiamlaluno on August 1, 2009 at 1:35pm
Status:needs review» needs work
  1. <?php
          '#title'         => l('ForexFeed.net', 'http://forexfeed.net', array('attributes'=>array('target'=>'_blank')) ) .' '. t('Access Key'),
    ?>

    I am not sure you can use HTML tags for the title of a textfield; it seems more appropriate to add a link in the description for the form field (or in the help for the page).

  2. forex_feed.admin.inc includes only a function; in that case it's probably better to move the code to the module file.
  3. <?php
    /**  ForexFeed.net Access Key  */
    function _forex_feed_get_access_key(){
      return     variable_get('forex_feed_access_key', '');
    }

    /**  Number of available Blocks  */
    function _forex_feed_get_num_blocks(){
      return     variable_get('forex_feed_num_blocks', 1);
    }
    ?>

    It would be better to use directly variable_get().

  4. The indentation used should be 2 spaces.

#4

Posted by ForexFeed on August 1, 2009 at 5:47pm

Thanks for the review. Admin function and the variable wrappers have been refactored.

AttachmentSize
forex_feed.tar_.gz 69.06 KB

#5

Posted by ForexFeed on August 1, 2009 at 5:47pm
Status:needs work» needs review

#6

Posted by kiamlaluno on August 1, 2009 at 7:43pm
Status:needs review» needs work
  1. Drupal coding standards are not completely followed.
  2. I would also remove the files that are not necessary in the repository (the license file, and the images that are not used in any places).

#7

Posted by ForexFeed on August 2, 2009 at 7:00pm
Status:needs work» needs review
AttachmentSize
forex_feed.tar_.gz 3.01 KB

#8

Posted by ForexFeed on August 2, 2009 at 7:12pm

Sorry, was missing the // $Id$ comment tag. Thanks again.

AttachmentSize
forex_feed.tar_.gz 3.02 KB

#9

Posted by kiamlaluno on August 2, 2009 at 7:13pm
Status:needs review» fixed

#10

Posted by System Message on August 16, 2009 at 7:20pm
Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

nobody click here