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.

Comments

ForexFeed’s picture

StatusFileSize
new69.17 KB
ForexFeed’s picture

Status: Postponed (maintainer needs more info) » Needs review
avpaderno’s picture

Status: Needs review » Needs work
  1.   '#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. /**  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.
ForexFeed’s picture

StatusFileSize
new69.06 KB

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

ForexFeed’s picture

Status: Needs work » Needs review
avpaderno’s picture

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).
ForexFeed’s picture

Status: Needs work » Needs review
StatusFileSize
new3.01 KB
ForexFeed’s picture

StatusFileSize
new3.02 KB

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

avpaderno’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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