Change private methods to protected in SwiftMailer class

+1 for this. Just had a similar use case. I needed to build up SMTP connection to a self-signed certificate. I'll probably open another issue suggesting to fire an event allowing to manipulate these parameters. But nevertheless it's not nice that - for whatever reason one might have - overriding the swiftmailer plugin results in duplicating all those private functions, just because they are private

I am fine with changing the private methods to protected, but this was done because these methods are not meant to be API or extension points.

Lets evaluate other solutions first:

#5 could be done via hook_swiftmailer_alter.

Regarding the file_get_contents problem: maybe we should replace file_get_contents with a proper http client, which then could be configured via dependency injection.

#5 could be done via hook_swiftmailer_alter.

That's correct, thanks!

Drupal core has 23 instances of "private function" and 1760 of "protected function". This suggests that the idea is that Drupal is customisable software - without the need for developers to come to the maintainers and "prove" that they have a valid customisation requirement.

Perhaps we could set the functions to protected with a comment that the maintainers reserve the right to change the interface without notice?

@webflo would you be happy with that?

Yes, protected methods are part of an inheritance API. There are quite some discussions around that in core, the tendency to being less open to "monkeypatching" by subclassing.

The methods we currently have look to me as quite stable, so i'd be fine with setting that to protected and giving API guarantee.
If there are strong objections about that, we can mark them protected, but also @internal, meaning "you can monkey-patch this, but do not rely on this".

#10 makes sense to me.

By default we won't add @internal, but @webflo (or anyone else) we can if you want. If so please set status back to "Needs Works" for that.

We can wait 1 week before we commit this.

Is there a general rule within Drupal on how binding protected functions are in an API contract manner? From my view, everything that's not defined in an interface, is potentially subject to change (even interfaces changes over time in contrib modules, but they should be at least planned as being stable and final). Of course I expect mentioning these changes in release notes and change records.

Imho every dev should anyway try other extension possibilities first, before swapping a service by subclassing the original. Often we have the possibility with plugins, events, alter hooks, etc. But in the end, sometimes your requirements are so custom to swap some services. And this is so great and powerful in D8, that you can do this so easily and often only involving a few lines of code

So I'd vote +1 for changing to protected. But why not leave an extra line comment in every signature we change here, that this is potentially subject to change

#13 Yes we could and I like the proposal in #10 that the line comment for "subject to change" would be @internal.

If anyone wants to post a patch adding @internal that's fine by me.

Probably massageMessageBody() should be protected not public??

makes sense

OK taking in the balance of opinion it seems like we should

• add @internal
• make massageMessageBody() protected also

Setting to "Needs work" for that.

LGTM.

Great thanks