Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The default and testing mail classes are both in system.mail.inc, there doesn't seem to be a particular reason for this and they could equally live in /core/lib/Drupal/Core.
Comment | File | Size | Author |
---|---|---|---|
#10 | mail-psr0-10.patch | 18.68 KB | amateescu |
#5 | mail-psr0-5.patch | 18.7 KB | amateescu |
#5 | interdiff-3.txt | 3.55 KB | amateescu |
#3 | mail-psr0-3.patch | 18.63 KB | amateescu |
#3 | interdiff.txt | 693 bytes | amateescu |
Comments
Comment #1
amateescu CreditAttribution: amateescu commentedI had a small problem with this patch, about not being able to define a
Default
class in Drupal\Core\Mail because of PHP restrictions, so in lack of any better name, I renamed the default class to.. *drumroll*.. Mail :) Basically, DefaultMailSystem is now Drupal\Core\Mail\Mail.I suppose we want to leave further clean-up of mail.inc to another issue? I'm thinking of a base class (abstract?) that will hold all the underscored functions that now reside in mail.inc.
Comment #3
amateescu CreditAttribution: amateescu commentedSigh..
Comment #4
Crell CreditAttribution: Crell commentedYeah, I agree, silly name. :-) Could we perhaps call this something descriptive, like Sendmail, or PhpMail?
We don't have a Drupal\Core\Cache\Cache class for the default, we have a Drupal\Core\Cache\Database class for the default. That is more descriptive.
This may not be the place to make this change, but "Testing" is a bad name for this class. It documents what it is often used for, not what it does. It should rather be something like VariableLog.
This looks good otherwise on visual inspection. Is there another inc file we can kill in the process? :-)
20 days to next Drupal core point release.
Comment #5
amateescu CreditAttribution: amateescu commentedI like PhpMail and VariableLog, so went ahead with those names. Interdiff is against #3.
The only inc file left is mail.inc, which still contains some procedural stuff, but I think converting that to OO deserves a new issue.
Comment #6
Crell CreditAttribution: Crell commentedI'm satisfied with #5, but it should probably get a second set of eyes on it before it goes RTBC.
Comment #7
catchBoth those class names look fine. mail.inc itself is definitely it's own issue.
I can't see anything to complain about here so I'm marking this RTBC, will leave it the usual 2-3 days before commit so people have a chance to object.
Comment #8
catch#5: mail-psr0-5.patch queued for re-testing.
Comment #10
amateescu CreditAttribution: amateescu commentedRerolled for the system.info fail.
Comment #11
catchCommitted/pushed to 8.x, thanks!