DB field for when a subscriber was originally added to the tables

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..83ca731
--- /dev/null
+++ b/.gitignoreundefined
@@ -0,0 +1,3 @@
+/.settings
+/.buildpath

That shouldn't be here :)

You can add the .gitignore to .gitignore, to ignore itself.

+++ b/simplenews.installundefined
@@ -748,3 +760,23 @@
+  ));
+  ¶
+  db_add_field('simplenews_subscriber', 'protocol', array(
+    'description' => 'Contains a record of changes the subscriber has made such as Date and action (e.g. 2012-04-13: unsubscribe from XYZ Newsletter).',

Trailing whitespace.

+++ b/simplenews.moduleundefined
@@ -904,6 +904,7 @@
         drupal_set_message(t('You have been subscribed to %newsletter.', array('%newsletter' => _simplenews_newsletter_name($newsletter))));
+        $subscriber->protocol[REQUEST_TIME] = REQUEST_TIME . ': ' . t('You have been subscribed to %newsletter.', array('%newsletter' => _simplenews_newsletter_name($newsletter)));

What happens if two changes happen in the same request? Also, REQUEST probably shouldn't be hardcoded into the protocol.

The problem here is translatability. Your code will translate the string and then save the translation into the database. This means that if the subscriber used chinese, you will see the log entry in chinese, which will not be helpful.

So we'd probably need a system like watchdog, where you store untranslated strings and arguments and translate is done when viewed. Requires some tricks to be able to extract the translatable strings, though like adding them to a dummy function.

Maybe a separate table might make sense, then we don't have to worry about duplicate protocol entries, could build a list of recent changes easily and don't have to deal with a possibly large protocol data every time we load a subscriber.

Well, administrators can also be talking chinese on a chinese website, so they want the messages translated.

I didn't mean actually using watchdog. I meant adding our own table that works similar to watchdog in the way that it stores an untranslated text and a serialized array for the parameters.

That table needs a serial key, reference to snid, a text and placeholders columns.

Agreed with splitting up the patches. Wondering if we shouldn't simply use created instead of timestamp_created, that's the defacto standard for that column. And timestamp should be changed, but that's another story.

Oh, crosspost with the bot.

[04:40:21] Command [git apply --check -p1 /var/lib/drupaltestbot/sites/default/files/review/simplenews-subscriber-insert-date-1531916-8.patch 2>&1] failed
Duration 0 seconds
Directory [/var/lib/drupaltestbot/sites/default/files/checkout/sites/default/modules/simplenews]
Status [128]
Output: [fatal: corrupt patch at line 48].

Sounds like something with your patch is wrong, maybe you tried to split it up manually?

The status you meant is "reviewed & tested by the community" (RTBC). Which you should not set yourself :)

As mentioned, I'd prefer to name the column just "created". Patch looks good to me otherwise.

Thanks, commited.