Well, this is just a minor issue that concerns on documentation consistency.

As described here, there should do not exist a hook_commerce_currency_info(), since Commerce is expected to define all the legal currencies.
Modules may use hook_commerce_currency_info_alter(&$currencies) to define possible needed currencies.

So, at commerce.module:249 there is:

    module_load_include('inc', 'commerce', 'includes/commerce.currency');
    $currencies = module_invoke_all('commerce_currency_info');
    drupal_alter('commerce_currency_info', $currencies);

In fact, commerce.currency.inc has a hook_commerce_currency_info() implementation.
So, any module could implement hook_commerce_currency_info(), contradicting the description above.

CommentFileSizeAuthor
#4 hook-commerce-currency-info-review-1108752-4.patch857 byteseltermann
removing-hook-commerce-currency-info.patch1.24 KBeltermann

Comments

yasminpibo’s picture

yasminpibo commented
rszrama’s picture

rszrama commented

Hmm, interesting. Your patch definitely fixes the disparity between the documentation and the code, but I wonder which is better... leave the code as is and change the documentation, or apply your patch so the code conforms to the documentation? I could go either way, really, but I have to wonder if we choose to just change the documentation for the sake of websites using fake / custom currencies (i.e. reward points, user points, etc.). As the documentation indicates, those can still be accommodated through currency alteration, but then you can introduce conflicts between modules implementing the currency alteration hook.

I'll get a second opinion from Damien. My hunch based on conversations with him about fake currencies is that he'd prefer to update the docs.

rszrama’s picture

rszrama commented
Status: Needs review » Needs work

Thought about this a little more. I think we should just update the documentation and document the hook in the .api.php file.

eltermann’s picture

eltermann commented
StatusFileSize
new hook-commerce-currency-info-review-1108752-4.patch857 bytes

commerce.api.php file has already the documentation of hook_commerce_currency_info() and hook_commerce_currency_info_alter().

The patch I'm posting is just to let array format as Drupal coding standards: http://drupal.org/coding-standards#array

I'll write a text to documentation of "Commerce info hooks" (http://www.drupalcommerce.org/specification/info-hooks/commerce) and then post it here.

eltermann’s picture

eltermann commented
Status: Needs work » Needs review

Text submitted on documentation: http://www.drupalcommerce.org/specification/info-hooks/commerce

rszrama’s picture

rszrama commented
Status: Needs review » Fixed

Great! I committed the patch and tweaked the docs just a little bit, as I had some other inaccurate information in there about currency info being objects vs. arrays (everything's an array now) and hadn't documented the conversion_rate property there yet.

Status: Fixed » Closed (fixed)

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