Review als module code withg coder and apply coding standards and hints where applicable.

Comments

corvus_ch’s picture

Status: Active » Needs review
StatusFileSize
new47.29 KB
berdir’s picture

Status: Needs review » Needs work

Awesome cleanup.

+++ b/includes/simplenews.admin.incundefined
@@ -138,10 +138,7 @@ function simplenews_admin_issues() {
   if (variable_get('simplenews_last_cron', '')) {
     $form['last_sent'] = array(
-      '#value' => '<p>' . format_plural(variable_get('simplenews_last_sent', 0),
-                                        'Last batch: 1 mail sent at !time.',
-                                        'Last batch: !count mails sent at !time.',
-                                        array('!time' => format_date(variable_get('simplenews_last_cron', ''), 'small'), '!count' => variable_get('simplenews_last_sent', 0))) . "</p>\n",
+      '#value' => '<p>' . format_plural(variable_get('simplenews_last_sent', 0), 'Last batch: 1 mail sent at !time.', 'Last batch: !count mails sent at !time.', array('!time' => format_date(variable_get('simplenews_last_cron', ''), 'small'), '!count' => variable_get('simplenews_last_sent', 0))) . "</p>\n",

As we're changing the line anyway. I've never seen this in the user interface before ;) This has the same issue as the language in the subscriber edit form. Should be #markup and not #value.

+++ b/includes/simplenews.subscription.incundefined
@@ -578,7 +578,7 @@ function simplenews_confirm_add_form($form, &$form_state, $mail, $newsletter) {
-  if (!$path =  variable_get('simplenews_confirm_subscribe_page', '')){
+  if (!$path =  variable_get('simplenews_confirm_subscribe_page', '')) {
     $path = variable_get('site_frontpage', 'node');
     drupal_set_message(t('%user was added to the %newsletter mailing list.', array('%user' => $form_state['values']['mail'], '%newsletter' => _simplenews_newsletter_name($form_state['values']['newsletter']))));

Two spaces after the =

+++ b/includes/simplenews.subscription.incundefined
@@ -666,7 +666,7 @@ function simplenews_confirm_removal_form($form, &$form_state, $mail, $newsletter
-  if (!$path =  variable_get('simplenews_confirm_unsubscribe_page', '')){
+  if (!$path =  variable_get('simplenews_confirm_unsubscribe_page', '')) {
     $path = variable_get('site_frontpage', 'node');

Same here.

+++ b/includes/simplenews.subscription.incundefined
@@ -725,7 +725,7 @@ function simplenews_subscriptions_admin_form($form, &$form_state, $snid) {
       $name = t($item->name);
-      $language_options[$langcode] = $name . ($item->native != $name ? ' ('. $item->native .')' : '');
+      $language_options[$langcode] = $name . ($item->native != $name ? ' (' . $item->native . ')' : '');
     }
     // std users have language in profile. disable
     $disabled = $subscriber->uid ? TRUE : FALSE;
@@ -735,11 +735,13 @@ function simplenews_subscriptions_admin_form($form, &$form_state, $snid) {

@@ -735,11 +735,13 @@ function simplenews_subscriptions_admin_form($form, &$form_state, $snid) {
       '#description' => t('The e-mails will be localized in language chosen. Real users have their preference in account settings.'),
       '#disabled' => FALSE,
     );
+    $language_default = language_default()->name;
     if ($subscriber->uid) {

Now that I think about this, let's use $language_options[language_default()->language] for $default_language, then it's consistently formatted (with the native name in ()).

+++ b/includes/views/handlers/simplenews_handler_field_newsletter_send.incundefined
@@ -1,5 +1,11 @@
 /**
+ * @file
+ *
+ * @todo Describe what this file is for.
+ */
+
+/**
  * Field handler to present a link to close or open commenting on a node.

"Views handler that displays a link to send a newsletter."

The class comment is copy pasted from somewhere and needs to be changed to the same.

corvus_ch’s picture

StatusFileSize
new50.2 KB
berdir’s picture

Status: Needs work » Needs review
corvus_ch’s picture

StatusFileSize
new50.26 KB
berdir’s picture

Status: Needs review » Fixed

Great stuff. Commited.

miro_dietiker’s picture

Great to see this fixed!

Status: Fixed » Closed (fixed)

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