Comments

Niklas Fiekas’s picture

Status: Active » Needs review
StatusFileSize
new72.44 KB
new6.74 KB

Classes converted.

I have also attached the concatenated output of git blame -C on the new files, showing that most of the code originated in the old file. Best viewed without automatic linewrapping, without Dreditor or zoomed out, to get an overview.

Niklas Fiekas’s picture

+++ b/core/modules/system/lib/Drupal/system/Tests/Mail/Test.phpundefined
@@ -0,0 +1,92 @@
+    variable_set('mail_system', array('default-system' => 'MailTestCase'));

Meh ... should have renamed this to the new class name.

aspilicious’s picture

I know it's bad to repeat "Mail" in the name of the classes. But the class "Test" looks even worse, it doesn't say what the class does.

options:
1) Find a better name for the class
2) Just name it MailTest, like we did with the tracker module and others. At least this tells us that we are testing mail functionality

Niklas Fiekas’s picture

Html to text conversion doesn't rely on mailing => Does not nescessarily belong into the Mail namespace => Now we have two classes, that don't qualify each for an own namespace => Just move them both to common. Also now we don't have Mail/Test or Mail/MailTest, but Common/MailTest instead.

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Good to go!

Niklas Fiekas’s picture

StatusFileSize
new21.22 KB

Thanks.

As sun suggested, here's also a diff with lower thresholds for rename detection.

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks!

jhodgdon’s picture

Uh oh. I just realized that this commit was not done correctly. I used patch -p1 to apply the patch, but it did not do the rename properly. So I had to do a second commit. I backed out the patch above with patch -p1 -R, and then used git apply to re-apply. The net result was:
rename core/modules/system/{tests/mail.test => lib/Drupal/system/Tests/Common/HtmlToTextTest.php}

So, now it's actually fixed. Sorry about that!

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