Update MailsystemManager::__construct() for 8.2.x compatibilty

To make the code work with both versions, what we need to do is write a constructor that now also receives the renderer dependency and then depending on a core version check (\Drupal::VERSION) calls either the old or the new constructor.

The same code should then work with both 8.1.x and 8.2.x.

+++ b/src/MailsystemServiceProvider.php
@@ -28,10 +28,17 @@ class MailsystemServiceProvider implements ServiceProviderInterface, ServiceModi
     // Overrides mail-factory class to use our own mail manager.
-    $container->getDefinition('plugin.manager.mail')
-      ->setClass('Drupal\mailsystem\MailsystemManager')
-      ->addArgument(new Reference('theme.manager'))
-      ->addArgument(new Reference('theme.initialization'));
+    $definition = $container->getDefinition('plugin.manager.mail');
+    // Replace the last plugin.manager.mail service identifier (renderer)
+    // provided since Core 8.2.
+    $definition->replaceArgument(6, new Reference('theme.manager'));
+    $definition->addArgument(new Reference('theme.initialization'));
+
+    if (version_compare(\Drupal::VERSION, '8.2', '>=')) {
+      $definition->addArgument(new Reference('renderer'));
+    }
+
+    $definition->setClass('Drupal\mailsystem\MailsystemManager');

Nice work, first thought the version check is strange, but what you have does make sense.

That said, I just had an idea that might make this easier and less prone to break again with future changes.

Instead of the arguments, we could inject our additional arguments with setter methods. You can add method calls to the definition, with references. So we'd have a setThemeNanager($theme_manager). Usually, you only do that for optional dependencies, but it might be a nice way out for us.

The advantage is that we can remove our own constructor code completely and don't have any version specific code anymore.

Lets try that. If it works out well, then I think I'll even open a core documentation issue to make that the recommended approach to avoid troubes with changes in minor versions.

Also, the patch actually failed in 8.1.x because that does not have the 6th argument. So you'd need to make the whole part there version specific.

Risky means there are no assertions. There ae no assertions, that's why it is risky, but that's a problem with this issue.

Improved the test a bit. our two classes now need to be set with the set methods, no longer through the constructor. That also means we don't need the version check there, see comment.

Oops, this should be better, interdiff is against #12 still.

Committed.