Follow-up to #950534: [policy] Consistently use "email" instead of "e-mail" in Drupal

Problem/Motivation

Back in #950534: [policy] Consistently use "email" instead of "e-mail" in Drupal we changed all occurrences of "e-mail" to "email". Since then a few occurrences of "e-mail" have crept in.

Proposed resolution

Fix it.

Remaining tasks

User interface changes

Change e-mail to email in admin/people/permissions descriptions and on the user/edit form and login block.

API changes

None.

Data model changes

None.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task
Issue priority Normal
Unfrozen changes Unfrozen because it only changes strings, mostly in comments and also in user interface as detailed above.
Prioritized changes Not a prioritized change.
Disruption Not disruptive.
CommentFileSizeAuthor
#36 follow_up_consistently-2551453-36.patch9.57 KBlomo
#33 follow_up_consistently-2551453-33.patch.patch9.57 KBlomo
#31 replace_e-mail_with_email-2551453-31.patch9.57 KBmadhavvyas
#24 2551453-interdiff-24-17.txt2.66 KBjaxxed
#24 2551453-24-replace_e-mail_with_email.patch9.57 KBjaxxed
#21 egrep.email_.txt4.86 KBjaxxed
#19 2551453-interdiff-17-7.txt5.04 KBjaxxed
#17 follow_up_consistently_e_mail_to_email-25514533-17.patch7.7 KBayesh
#10 follow_up_consistently-2551453-8.patch12.12 KBlomo
#5 follow_up_consistently_e-mail_to-email-2551453-5.patch12.67 KBayesh
#2 2551453.patch6.87 KBlinl

Comments

LinL created an issue. See original summary.

linl’s picture

linl commented
Status: Active » Needs review
StatusFileSize
new 2551453.patch6.87 KB
linl’s picture

linl commented
Status: Needs review » Needs work
Issue tags: +Needs reroll, +Novice

No longer applies, tagging for reroll.

Maybe a good novice issue for Barcelona sprint?

ayesh’s picture

ayesh commented
Assigned: Unassigned » ayesh
ayesh’s picture

ayesh commented
StatusFileSize
new follow_up_consistently_e-mail_to-email-2551453-5.patch12.67 KB

Not sure if this issue was only about the user module, but this patch fixes more than 2 references.

ayesh’s picture

ayesh commented
Assigned: ayesh » Unassigned
Status: Needs work » Needs review
lomo’s picture

lomo commented
Status: Needs review » Needs work

Good. You are right, Ayesh, that there are quite a few instances of "e-mail" to fix outside of the user.module. And the patch looks good to me... except for one little thing:

		diff --git a/modules/devel/src/Plugin/Mail/DevelMailLog.php b/modules/devel/src/Plugin/Mail/DevelMailLog.php
index d29ec7f..27c1a1e 100644
--- a/modules/devel/src/Plugin/Mail/DevelMailLog.php
+++ b/modules/devel/src/Plugin/Mail/DevelMailLog.php
@@ -68,7 +68,7 @@ private function dirify($string) {
   }
 
   /**
-   * Save an e-mail message to a file, using Drupal variables and default settings.
+   * Save an email message to a file, using Drupal variables and default settings.
    *
    * @see http://php.net/manual/en/function.mail.php
    * @see drupal_mail()

Looks like you searched/replaced in the project, but also included a contrib module (Devel), so the patch does not apply. The fix is trivial, of course...

ayesh’s picture

ayesh commented
StatusFileSize
new follow_up_consistently_e_mail_to_email-25514533-7.patch11.79 KB

Sorry you are right I had it replaced in devel too. Thanks Lowell. Attaching an update :)

ayesh’s picture

ayesh commented
Status: Needs work » Needs review
lomo’s picture

lomo commented
StatusFileSize
new follow_up_consistently-2551453-8.patch12.12 KB

LOL... I was just doing the same. But I guess that means I can still review. ;-)

lomo’s picture

lomo commented

Damn... I thought I'd got rid of the attachment I uploaded. It should just be your patch.

lomo’s picture

lomo commented
Status: Needs review » Reviewed & tested by the community

I think that looks good now and we should just get this into core before it needs another re-roll. Marking it RTBC. Nice work, Ayesh. :-)

The last submitted patch, 5: follow_up_consistently_e-mail_to-email-2551453-5.patch, failed testing.

linl’s picture

linl commented
Status: Reviewed & tested by the community » Needs work

Thanks Ayesh and LoMo! Patch is looking good, but any instances in files in core/modules/migrate_drupal should be left as they are as those files are generated by the migrate process.

See for example: core/modules/migrate_drupal/src/Tests/Table/d6/MenuRouter.php

/**
 * @file
 * Contains \Drupal\migrate_drupal\Tests\Table\d6\MenuRouter.
 *
 * THIS IS A GENERATED FILE. DO NOT EDIT.
 *
 * @see core/scripts/migrate-db.sh
 * @see https://www.drupal.org/sandbox/benjy/2405029
 */

Setting to Needs work to exclude the migrate files.

The last submitted patch, 10: follow_up_consistently-2551453-8.patch, failed testing.

The last submitted patch, , failed testing.

ayesh’s picture

ayesh commented
StatusFileSize
new follow_up_consistently_e_mail_to_email-25514533-17.patch7.7 KB

Trying again. Thanks Lin.

ayesh’s picture

ayesh commented
Status: Needs work » Needs review
jaxxed’s picture

jaxxed commented
StatusFileSize
new 2551453-interdiff-17-7.txt5.04 KB

quick interdiff 17-8 (which was labelled 7)

linl’s picture

linl commented
Status: Needs review » Needs work

Thanks Ayesh. Nearly there, I think. I just noticed (sorry missed it in my last review) that a couple of e-mails to emails changes in the original patch in #2 are missing.

Here:

+++ b/core/lib/Drupal/Core/Render/RendererInterface.php
@@ -42,7 +42,7 @@ public function renderRoot(&$elements);

@@ -42,7 +42,7 @@ public function renderRoot(&$elements);
    *
    * Calls ::render() in such a way that placeholders are replaced.
    *
-   * Useful for e.g. rendering the values of tokens or e-mails, which need a
+   * Useful for e.g. rendering the values of tokens or emails, which need a

and:

+++ b/core/modules/user/user.permissions.yml
@@ -3,11 +3,11 @@ administer permissions:
 administer users:
   title: 'Administer users'
-  description: 'Manage all user accounts. This includes editing all user information, changes of e-mail addresses and passwords, issuing e-mails to users and blocking and deleting user accounts.'
+  description: 'Manage all user accounts. This includes editing all user information, changes of email addresses and passwords, issuing emails to users and blocking and deleting user accounts.'
   restrict access: true
jaxxed’s picture

jaxxed commented
StatusFileSize
new egrep.email_.txt4.86 KB

here is a grep result list (post #17)

relevant possible targets:

./core/lib/Drupal/Core/Render/RendererInterface.php:45:   * Useful for e.g. rendering the values of tokens or e-mails, which need a
./core/modules/simpletest/src/InstallerTestBase.php:162:      // tests from sending out e-mails and collect them in state instead.
./core/modules/user/user.permissions.yml:6:  description: 'Configure site-wide settings and behavior for user accounts and registration. This includes account cancellation methods, the content of user e-mails and fields attached to users.'
./core/modules/user/user.permissions.yml:10:  description: 'Manage all user accounts. This includes editing all user information, changes of email addresses and passwords, issuing e-mails to users and blocking and deleting user accounts.'

EDIT: removed symfony vendor translations from target list as obviously we aren't responsible for that.

jaxxed’s picture

jaxxed commented

@Linl what to do with translation targets? Some of the may require approval for noun declination changes (cs, et, nl, tr/)

EDIT: ignore me

jaxxed’s picture

jaxxed commented

Let's also note that a number of these changes are going to change deep core Drupal files, and may mean that plenty of other patches are going to require re-rolling. let's be careful with this one.

jaxxed’s picture

jaxxed commented
StatusFileSize
new 2551453-24-replace_e-mail_with_email.patch9.57 KB
new 2551453-interdiff-24-17.txt2.66 KB

patch from #17 with a few more cases added.

jaxxed’s picture

jaxxed commented
Assigned: Unassigned » jaxxed
Status: Needs work » Needs review
jaxxed’s picture

jaxxed commented
Issue tags: -Needs reroll

The last submitted patch, 5: follow_up_consistently_e-mail_to-email-2551453-5.patch, failed testing.

The last submitted patch, 8: follow_up_consistently_e_mail_to_email-25514533-7.patch, failed testing.

The last submitted patch, 10: follow_up_consistently-2551453-8.patch, failed testing.

linl’s picture

linl commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Thanks jaxxed. I think that has got them all now - I grepped for e-mail, e-mails, E-mail, E-mails, E-Mail, and E-Mails.

But now it needs another reroll following #2570355: Replace remaining !placeholder and @placeholder with :placeholder for URLs

madhavvyas’s picture

madhavvyas commented
Issue tags: -Needs reroll
StatusFileSize
new replace_e-mail_with_email-2551453-31.patch9.57 KB

Patch re-rolled #24

madhavvyas’s picture

madhavvyas commented
Status: Needs work » Needs review
lomo’s picture

lomo commented
StatusFileSize
new follow_up_consistently-2551453-33.patch.patch9.57 KB

Wow... so many more turned up with all the variations.

error: patch failed: core/modules/user/src/AccountForm.php:151
error: core/modules/user/src/AccountForm.php: patch does not apply

And now the patch doesn't apply, but I think I've fixed it; it was only one character changed elsewhere in one line.

I was hoping to see this back to RTBC, but I guess someone else needs to review this one again. ;-)

Status: Needs review » Needs work

The last submitted patch, 33: follow_up_consistently-2551453-33.patch.patch, failed testing.

madhavvyas’s picture

madhavvyas commented
Status: Needs work » Needs review
lomo’s picture

lomo commented
StatusFileSize
new follow_up_consistently-2551453-36.patch9.57 KB

I managed to mess up something in that last patch... sorry. This should do it.

linl’s picture

linl commented

Latest patch is looking good. I can't really RTBC it as I did the original patch, but I think it is ready. (And we should wait for the testbot to come back green.)

The last submitted patch, 33: follow_up_consistently-2551453-33.patch.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 36: follow_up_consistently-2551453-36.patch, failed testing.

Status: Needs work » Needs review

LinL queued 36: follow_up_consistently-2551453-36.patch for re-testing.

lomo’s picture

lomo commented
Assigned: jaxxed » Unassigned

And I cannot RTBC, since I did the last patch, but it passed (at that time). Hopefully it still applies. I'm going to change to unassigned since this hasn't moved for some days and is a fairly simple review issue that could be finished today.

Miraya’s picture

Miraya commented

I am working on this at the mentored sprint in Barcelona 2015. My mentor is jp.stacey . I am reviewing the patch as per https://www.drupal.org/patch/review

linl’s picture

linl commented

Thanks Miraya! And J-P. (Missing you all, wish I was there at the sprint too :-) Will follow along from here!)

Miraya’s picture

Miraya commented
Status: Needs review » Reviewed & tested by the community

I reviewed the patch, it applies perfectly fine.
Also tested via grepping for 'e-mail', 'E-Mail', 'e-Mail', 'E-mail' as well as the plural forms. Nothing missing, nothing else is changed.

NikitaJain’s picture

NikitaJain commented

follow_up_consistently-2551453-36.patch patch no longer applies

Miraya’s picture

Miraya commented

@nikitajain: Patch applied fine on commit 1d1fe19702f5dc6894acc45431b1d0c85ab306d7. Can you give details to reproduce?

yesct’s picture

yesct commented

  • webchick committed 4efd3aa on 8.0.x 
    Issue #2551453 by LoMo, Ayesh, jaxxed, LinL, madhavvyas, Miraya: Follow-...
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed LIVE at DrupalCon Barleona! :D

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Issue tags: +Barcelona2015
yesct’s picture

yesct commented
Related issues: +#950534: [policy] Consistently use "email" instead of "e-mail" in Drupal

in #47 I had added a related issue... but d.o ate my changes. noting that so we can investigate.

ayesh’s picture

ayesh commented

Gracias everyone!

madhavvyas’s picture

madhavvyas commented

Great work!

Status: Fixed » Closed (fixed)

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