Patch proposal (mainly header management and From values)

You have not included a patch. See here for more info: http://drupal.org/patch/create

Also, please include a list of the other contrib modules you are using.

I'll wait.

Patch is malformed. I rerolled it using cvs diff.

Reroll for better coding standards compatibility and to reduce the number of comment changes: that's too much noise for this already big patch.

My last patch also converted tabs to spaces: I thought it was open-keywords who had used tabs instead of spaces, but apparently the original developer used some too… So this patch is actually larger than the previous one. But many of the added changes are trivial, so it should not make reviewing it too difficult.

Any chance we could get the split phpmailer patch in at the same time:

http://drupal.org/node/224849

That would help reduce the size of the main module and make reviewing less difficult.

-- Jason

It looks like the proposed patch (I'm currently looking at the one in #7) is addressing multiple issues. Are the SSL settings affecting whether the e-mail is sent or not? I prefer clean patches that only address one issue at a time. That way, it's easier to read through and figure out if everything's okay, and if there's a problem, it's easier to isolate and correct. The way it looks now, there's multiple bug fixes and code style changes all in one patch.

@open-keywords, based on the second "patch" you submitted, it looks to me like you didn't read the information on the link I sent you in my first comment.

@Wim Leers, while I agree that this module should use proper Drupal coding style, it still makes it difficult to read through and determine whether a change was done for the bug at hand or for code style, especially for a patch that seems to be fixing multiple problems. Also, I would like to submit a code style change that is a single, separate CVS submission, so all the noise is kept in one non-bug-fixing, non-feature-adding CVS submission.

@jsm174, not likely. I want fewer bug fixes/feature additions per patch, not more. I want to do the same thing so I will include it, but it will be a separate submission.

@oadaeh: I absolutely agree. See: "reduce the number of comment changes: that's too much noise for this already big patch."
If I had the time, I'd go through it again.

I committed the meat of this patch here: http://drupal.org/cvs?commit=106705

Please test it to make sure it does for you what you want as soon as you can so I can proceed with the other things I want to do with this module (splitting, complying and tagging). I want to make sure I got it right and that it doesn't break other things before moving on.

@open-keywords, thanks for the feedback. I'm going to mark this as fixed and proceed forward. If any problems show up, feel free to re-open this issue.