Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symfony Mailer Support #219

Merged
merged 1 commit into from Nov 27, 2019
Merged

Symfony Mailer Support #219

merged 1 commit into from Nov 27, 2019

Conversation

@Baachi
Copy link
Contributor

Baachi commented Nov 23, 2019

Hey Guys!

I created a symfony mailer reporter because swiftmailer is "official" dead I would think.
The SwiftMailer reporter is still available, so no BC breaks :)

#SymfonyHackday

@Baachi Baachi changed the title Add support for symfony 5 :rocket: Symfony Mailer Support Nov 23, 2019
@Baachi

This comment has been minimized.

Copy link
Contributor Author

Baachi commented Nov 25, 2019

Okay it seems that I have to adjust the Travis file. The mailer component does not run on php versions lower than 7.3. :/

@kbond

This comment has been minimized.

Copy link
Collaborator

kbond commented Nov 26, 2019

Thanks @Baachi, the travis issue is not with php < 7.3 but that symfony/mailer requires symfony components 4.4+.

Copy link
Collaborator

kbond left a comment

Looks good but I'd like declare(strict_types=1); removed - I'd like to follow the Symfony code standards.

@@ -0,0 +1,11 @@
<?php
declare(strict_types=1);

This comment has been minimized.

Copy link
@kbond

kbond Nov 26, 2019

Collaborator

Can you remove this?

@@ -0,0 +1,99 @@
<?php
declare(strict_types=1);

This comment has been minimized.

Copy link
@kbond

kbond Nov 26, 2019

Collaborator

remove.

@@ -0,0 +1,50 @@
<?php
declare(strict_types=1);

This comment has been minimized.

Copy link
@kbond

kbond Nov 26, 2019

Collaborator

Remove.

@@ -0,0 +1,65 @@
<?php
declare(strict_types=1);

This comment has been minimized.

Copy link
@kbond

kbond Nov 26, 2019

Collaborator

Remove.

Copy link
Collaborator

kbond left a comment

I made some changes to the travis config to better handle different symfony versions.

  1. Please rebase
  2. See https://github.com/doctrine/DoctrineBundle/blob/f6c5c7678b0ad3a6af8478e7903f3de3beeaf258/.travis.yml#L42 to see how we can add the mailer only on tests using symfony 4.3+ (I can do this step if you'd like)
try {
$mailerDefinition = $container->findDefinition('mailer');
} catch (ServiceNotFoundException $e) {
throw new MissingPackageException('To enable mail reporting you have to install the "swiftmailer/swiftmailer" or "symfony/mailer".');

This comment has been minimized.

Copy link
@kbond

kbond Nov 26, 2019

Collaborator

Can we just throw a \LogicException here? Don't know that we need a custom exception for this.

@@ -103,6 +112,48 @@ public function testMailer()
$this->assertContainerBuilderHasService('liip_monitor.reporter.swift_mailer');
}
public function testSymfonyMailer()

This comment has been minimized.

Copy link
@kbond

kbond Nov 26, 2019

Collaborator

mark as skipped if MailerInterface does not exist.

@@ -42,7 +42,8 @@
"symfony/asset": "^3.4|^4.0",
"symfony/templating": "^3.4|^4.0",
"phpunit/phpunit": "^7.0",
"symfony/finder": "^3.4|^4.0"
"symfony/finder": "^3.4|^4.0",
"symfony/mailer": "^4.3 || ^5.0"

This comment has been minimized.

Copy link
@kbond

kbond Nov 26, 2019

Collaborator

Remove this, we'll add with travis if symfony > 4.3

@kbond kbond force-pushed the Baachi:feature/symfony-mailer branch from ad4ec29 to d4f508f Nov 27, 2019
@kbond
kbond approved these changes Nov 27, 2019
@kbond kbond force-pushed the Baachi:feature/symfony-mailer branch from d4f508f to 8a1da1e Nov 27, 2019
@kbond

This comment has been minimized.

Copy link
Collaborator

kbond commented Nov 27, 2019

Thanks @Baachi, I made these changes.

@kbond kbond merged commit 91c7032 into liip:master Nov 27, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.