Stop logging comment IP addresses by default

Lovely! Works as expected on a new site.

Since we're changing a default, we need a change record, yes? Oh yes, you noted that in the description. Tagging accordingly.

I got:

Fatal error: Cannot redeclare comment_update_8301() (previously declared in core/modules/comment/comment.install:200)

so I changed it to 8401 (right?) and replicated the @addtogroup updates stuff (which I don't understand).

Also, any chance of an idiot's guide (in issue summary perhaps) for how to upgrade an existing site to use this? i.e. how do you get the new config setting in there without uninstalling and reinstalling the comment module (which you can't).

Thanks.

+++ b/core/modules/comment/comment.install
@@ -208,3 +208,23 @@ function comment_update_8301() {
+                ->save(TRUE);

Indentation is incorrect here.

Re-rolled (and resolved the indentation issue highlighted in #10).

Happy to see this become the better new default.

Add a configuration setting to control whether IP addresses are logged or not.

This does not provide a UI right? I couldn't find any in simplytest :-)

TBH, I hope the IP log option is default off and no UI to change it.

Keeping it only to be changed by who somehow really "needs" it would avoid for example a beginner developer that doesn't have a well configured server to set this option on - by obfuscation.

Agreed. To be clear: I was checking, not promoting a UI for it :)

Patch to prevent logging

It needs changes in \Drupal\Tests\comment\Kernel\CommentHostnameTest::testGetDefaultHostname (should fail now)
And upgrade path changes

Fixed test, now only upgrade path tests needed

no intention to provide a UI here

yes, UI should live in contrib a-la gdpr_comment and the URL /admin/structure/comment/settings

Fix token test

Added upgrade test

I guess it needs Lee to approve

Fix test for real

+++ b/core/modules/comment/comment.install
@@ -207,3 +207,13 @@ function comment_update_8600() {
+function comment_update_8601() {

this needs to be a post update hook as it is changing config

Crediting jibran who I confirmed that with

+++ b/core/modules/comment/comment.install
@@ -207,3 +207,4 @@ function comment_update_8600() {
+

nit: whitespace

looks good to me

fixed on commit

diff --git a/core/modules/comment/comment.install b/core/modules/comment/comment.install
index a77686c40e..82b578b489 100644
--- a/core/modules/comment/comment.install
+++ b/core/modules/comment/comment.install
@@ -207,4 +207,3 @@ function comment_update_8600() {
   $field_storage_definition->setDefaultValueCallback(Comment::class . '::getDefaultHostname');
   $entity_definition_update_manager->updateFieldStorageDefinition($field_storage_definition);
 }
-

Committed 36ecfc6 and pushed to 8.7.x.

Updated and published change record

Just came out at this post and sadly realised that ips are not stored in comment_field_data table any more. Is there a guide how to enable this feature (at least programmaticaly)? It is essential for my blog to store comments which are needed in the future.