Hello.

I was just looking over some proposed From values patches at http://drupal.org/node/224601?mode=2&sort=2

The proposed patch was a diff that contained all the lines from both the original and updated file. Since the PHPMailer and SMTP classes are both included inside smtp.module, it was a lot of code to look through -- not the end of the world, but you know what I mean.

Anyway, this proposed patched splits out PHPMailer and the SMTP classes into phpmailer.inc. This file will get included only once in the drupal_mail_wrapper() method.

There are no other changes in this patch from dev. It's just to help out with maintainability.

-- Jason

CommentFileSizeAuthor
smtp_split.patch170.01 KBjsm174

Comments

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Big +1 from me.

Subscribing.

open-keywords’s picture

open-keywords commented

Well, IMHO, since this branch of the PHPMailer classes has been stable for a few years, I don't see the need for a change.
And since the integration isn't exactly trivial, the PHPMailer's source can't be interchanged on the fly (having separate files could lead think it is the case)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Huh? We're only asking to move that huge class into a separate file, that's all…

jsm174’s picture

jsm174 commented

open-keywords,

I'm sorry, but I have to disagree. The purpose of splitting isn't so it would be easier to maintain the PHPMailer portion of the code. The PHPMailer and SMTP classes shouldn't really be changed anyway unless there is a bug or an update to those classes.

However, IMHO, I would think Drupal modules should be small and compact. Everytime the SMTP module is loaded, no matter what, it has to parse the PHPMailer and SMTP classes. With the patch provided, you get two benefits. 1) a smaller, more maintainable Drupal module and 2) two very large PHP classes that are only pulled in at the time of sending an email.

If it's a question of getting a patch applied, I would be more than willing to learn the Drupal patch procedure. It's something I have wanted to do for quite some time.

-- Jason

oadaeh’s picture

oadaeh commented
Status: Active » Needs work

I will do this, but I want to have the patch(es) from http://drupal.org/node/224601 included first. Then I'll do a code clean up. Don't worry about submitting a corrected patch (based on http://drupal.org/patch/create), as it's a straightforward change.

oadaeh’s picture

oadaeh commented

Upon further investigation into this issue, and based on http://drupal.org/node/66113, I've decide to totally delete the two classes from the module and have people download the PHPMailer package from http://phpmailer.codeworxtech.com/ instead.

My preliminary tests have shown that it will work, but I'm going to do a bit more testing before I make the change.

smk-ka’s picture

smk-ka commented

Is this (and other changes) still going to be backported to D5? The D5 and D6 branches are quite out of sync, that's why I'm asking.

oadaeh’s picture

oadaeh commented

@smk-ka: yes, my plan is to back port most of the changes (not all of them are compatible) done in the 6.x branch. I'm waiting until I feel I have a fairly stable, bug-free release, however, so that I'm not back-porting the bugs as well.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Closed (won't fix)

The Drupal 5 version of this module is no longer maintained.