mikaelli [mikaelli]

Hello, and thanks for applying for a CVS account. I am adding the review tags, and some volunteers will review your code, pointing out what needs to be changed.

As per http://drupal.org/cvs-application/requirements, the motivation message should be expanded to contain more details about the features of the proposed module, and it should include also a comparison with the existing solutions.

I will review the code tomorrow morning (about 24 hours from now).

1. The module doesn't contain the uninstallation function that should remove the Drupal variables defined from the module when it is going to be uninstalled.

2.   $t=
       '<img src="http://www.payson.se/images/logos/payson.gif" />'.
       t('Payson - Betala med kreditkort eller internetbank.');
   

   The string passed to t() must be in American English, which is the default language used from Drupal.

Hello!

I would very much appreciate if this module was completed, i'm writing this in an attempt to motivate limikael :) This module is wanted! Here's cheering for you.

Hej Mikael

I tried your module with payson.se in test mode and it seems everything is working but the ExtraCost variable, that for some reason is noted as missing in the Test page. I would be extremely grateful if you could help me out somehow, as I need a payment module very badly!

What could be wrong? Am I missing some flatrate settings or something?

HUGE thanks for your work!

• The points reported in this review are not in order or importance / relevance.
• Most of the times I report the code that present an issue. In such cases, the same error can be present in other parts of the code; the fact I don't report the same issue more than once doesn't mean the same issue is not present in different places.
• Not all the reported points are application blockers; some of the points I report are simple suggestions to who applies for a CVS account. For a list of what is considered a blocker for the application approval, see CVS applications review, what to expect. Keep in mind the list is still under construction, and can be changed to adapt it to what has been found out during code review, or to make the list clearer to who applies for a CVS account.

1.   global $conf;
     $conf['i18n_variables'][] = 'uc_payson_checkout_button';
   
   

   The global variable `$conf` is accessed through variable_get(), variable_set(), and variable_del().

2.   // Remove all our variables.
     db_query("DELETE FROM {variable} WHERE name LIKE 'uc_payson_%%'");
   }
   

   The code could delete Drupal variables defined from the module uc_payson_extension.module. Drupal variables should removed using variable_del(), passing to the function the name of each variables.

3. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how the code should be formatted.

I am closing this issue due to lack of replies.

Please read the following links as this is very important information about CVS applications.

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for these applications. Please read Migrating from CVS Applications to (Git) Full Project Applications and Applying for permission to opt into security advisory coverage on how this affects and benefits you and the application process. In short, every user has now the permissions necessary to create new projects, but they need to apply for opt into security advisory coverage. Without applying, the projects will have a warning on projects that says:

This project is not covered by Drupal’s security advisory policy.