Skip to content

Conversation

@limenet
Copy link

@limenet limenet commented Oct 24, 2016

When upgrading to Laravel 5.3 I encountered a warning about the queue used so I decided to write a patch (i.e. copy the default MailServiceProvider) to add compatibility with Laravel 5.3. This package now also supports a universal "to" address.

I did not test this PR against older Laravel versions, so I'd suggest marking this a breaking change.

Let me know if you'd like me to make any changes!

@brendandebeasi
Copy link

Can somebody please merge this in?

@stojanvujkov
Copy link

Please merge to support Laravel 5.3

@hillelcoren
Copy link

This change is important to us.

The main benefit we get from this package is we're able to easily get the messageId which we then use for open and bounce tracking. If we were to switch to the PHP library we wouldn't be able to use Laravel's Mail class.

@jaketoolson
Copy link

Beep beep! +1

@atheken
Copy link
Contributor

atheken commented Apr 18, 2017

@limenet - is it possible to fix the whitespace to match the original? I'm sure the changes are minor, but looks like the whole Provider class was modified.

@limenet
Copy link
Author

limenet commented Apr 18, 2017

@atheken sure! This was most likely caused by Sublime Text converting all tabs to spaces... I went through the file and reverted all whitespace-related modifications. Anything else you'd like me to change?

@atheken
Copy link
Contributor

atheken commented Apr 18, 2017

@limenet - Thanks, can you tell me if these changes are backwards compatible? Or should we update the composer.json to reflect that it only works with 5.3+?

@limenet
Copy link
Author

limenet commented Apr 18, 2017

@atheken I'm afraid I don't know that. When I made this PR, I noticed the provider wasn't compatible, googled for a solution, and - based on these findings - created this PR and it was compatible with Laravel 5.3. And - due to this package being sunset (and thus using the SMTP interface) - I don't know anything about compatibility with Laravel 5.4 either, but I assume it's compatible.

I would suggest, however, to also update composer.json just to be on the safe side. And since Laravel doesn't follow semantic versioning completely and an upgrade from 5.3 to 5.4 (or 5.2 to 5.3) is a manual task and not necessarily backwards compatible, I don't think updating composer.json would impact users negatively.

Requiring"illuminate/mail" as "^5.3" would be a good idea, I think. Or what would you suggest?

@hillelcoren
Copy link

@limenet Thanks for the fix, it works great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants