only adds functionality that got lost

Then we need to be able to test that added new functionality. I guess without some testing it is difficult to say if it should get merged or not.

Read the method declarations and the calls to the parent method. That makes it very clear that we aren't passing the data up the chain.

JMail is a massive trainwreck because it completely breaks the inherited API.

@mbabker Exactly!

@laoneo
The fact is that the JMail method signature is the same as the PHPMailer parent. The small value that JMail's method add, is, that several attachments can be added by passing an array of filenames/filepaths.

What JMail screws up is that the last parameter $disposition does not get passed up the chain.
As such this parameter is always the default value of PHPMailer ('attachment').

Thus JMail effectively removes functionality and flexibility that PHPMailer offers.

Then we need to be able to test that added new functionality. I guess without some testing it is difficult to say if it should get merged or not.

I don't consider it new functionality as it is / was there in PHPMailer all the time.
The reason I don't see tests as necessary is, that no Joomla code is using that last parameter.

I absolutely agree on your points. But as soon as a change has a different behavior than before the patch I suggest to write up some testing instructions. That's my opinion.

Pre-patch - Argument of the method is fully ignored

Post-patch - Argument of the method is properly passed forward through the call chain

Simple review - RTC