module under review

Good to hear. FYI I posted on the module review.

Thanks

Note: I'm going to merge those changes in separate commits to have a better history but we're using this issue to do the review of those changes.

+++ b/includes/commerce_worldpay_bg.page.incundefined
@@ -91,7 +89,7 @@ function commerce_worldpay_bg_response_page($payment_method = NULL, $debug_wppr
-      if ($full_log)
+      if ($full_log) {
         watchdog(
           'commerce_worldpay_bg',
           'Request with no transId was sent from <em>@ip</em> with request method <b>@method</b>. Refered by: <b>@referer</b>.',
@@ -101,12 +99,13 @@ function commerce_worldpay_bg_response_page($payment_method = NULL, $debug_wppr

@@ -101,12 +99,13 @@ function commerce_worldpay_bg_response_page($payment_method = NULL, $debug_wppr
             '@referer' => !empty($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : 'UNKNOWN',
           ),
           WATCHDOG_NOTICE);
-      return;
+        return;

This is not the same, the return should be outside of the inner diff, same below.

+++ b/theme/commerce-worldpay-bg-success.tpl.phpundefined
@@ -53,7 +55,7 @@
-  <WPDISPLAY ITEM=banner>
+    <WPDISPLAY ITEM=banner>

Not sure why this was changed?

+++ b/includes/commerce_worldpay_bg.page.incundefined
@@ -281,18 +282,17 @@ function _commerce_worldpay_bg_payment_response_authenticate($order_wrapper, &$w
-    watchdog('commerce_worldpay_bg', 'Access denied! ' . $message . ' Clients details: <em>@ip</em> with request method <b>@method</b>. Refered by: <b>@referer</b>.', 
...
-        '@referer' => !empty($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : 'UNKNOWN'
+        '@referer' => !empty($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : 'UNKNOWN',

Watchdog always adds the referer as a separate column, not need to duplicate this.

+++ b/includes/commerce_worldpay_bg.page.incundefined
@@ -281,18 +282,17 @@ function _commerce_worldpay_bg_payment_response_authenticate($order_wrapper, &$w
+    $ip = ip_address();
...
-        '@ip' => !empty($_SERVER['REMOTE_ADDR']) ? $_SERVER['REMOTE_ADDR'] : '0.0.0.0',
+        '@ip' => !empty($ip) ? $ip : '0.0.0.0',

I don't think the fallback here is necessary, ip_address() always returns something.

+++ b/theme/commerce-worldpay-bg-success.tpl.phpundefined
@@ -34,7 +34,6 @@
- * <span><?php print t('Merchant\'s Reference:'); ?>&nbsp;</span><span><b></b></span><br>

That's part of the todo, let's not remove that.

Ok, committed the changes and pushed to https://drupal.org/project/commerce_worldpay

Will also post in the project application issue.