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

[Mailer] Change DSN syntax #33494

Merged
merged 1 commit into from Sep 7, 2019

Conversation

@fabpot
Copy link
Member

commented Sep 6, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR -

The current syntax for failover and roundrobin is confusing. && and || do not really convey the right meaning. I realized that while working on a new transport that will send on more than one transport in parallel. && would be a natural fit, but that's already taken.

So, this pull request changes the syntax to be more explicit.

@fabpot fabpot changed the title Mailer change dsn syntax [Mailer] Change DSN syntax Sep 6, 2019

@fabpot fabpot force-pushed the fabpot:mailer-change-dsn-syntax branch 2 times, most recently from c29fc96 to aec11ea Sep 6, 2019

@fabpot

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2019

This PR also allows to nest transports.

@nicolas-grekas nicolas-grekas added this to the next milestone Sep 6, 2019

@fabpot fabpot force-pushed the fabpot:mailer-change-dsn-syntax branch from aec11ea to 4408910 Sep 6, 2019

@stof

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

would be great to add tests covering the error cases of the parsing (unclosed parenthesis, nesting a parenthesis directly in the argument, passing an unknown function name, garbage at the end, etc...)

@fabpot

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2019

@stof I've added some more tests.

@fabpot fabpot force-pushed the fabpot:mailer-change-dsn-syntax branch from 4408910 to f30dc2b Sep 7, 2019

@fabpot fabpot force-pushed the fabpot:mailer-change-dsn-syntax branch from f30dc2b to 39dd213 Sep 7, 2019

@fabpot fabpot referenced this pull request Sep 7, 2019
fabpot added a commit that referenced this pull request Sep 7, 2019
feature #33494 [Mailer] Change DSN syntax (fabpot)
This PR was merged into the 4.4 branch.

Discussion
----------

[Mailer] Change DSN syntax

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | yes
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | -

The current syntax for failover and roundrobin is confusing. `&&` and `||` do not really convey the right meaning. I realized that while working on a new transport that will send on more than one transport in parallel. `&&` would be a natural fit, but that's already taken.

So, this pull request changes the syntax to be more explicit.

Commits
-------

39dd213 [Mailer] Change the syntax for DSNs using failover or roundrobin

@fabpot fabpot merged commit 39dd213 into symfony:4.4 Sep 7, 2019

2 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

@fabpot fabpot deleted the fabpot:mailer-change-dsn-syntax branch Sep 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.