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

[Reference] Add missing option delivery_whitelist #4925

Closed
wants to merge 1 commit into from

Conversation

e-weimann
Copy link
Contributor

Q A
Doc fix? yes
New docs? no
Applies to SwiftMailerBundle 2.1
Fixed tickets no


**type**: ``array``

If set, only email message send to addresses that match with the values in the list.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this line should be replaced with:

Used in combination with ``delivery_address``. If set, send email as normal to addresses that match with the values in the list. All addresses not matching will be sent to ``delivery_address`` as is the normal behaviour when setting a ``delivery_address``.

@TerjeBr
Copy link
Contributor

TerjeBr commented Jan 26, 2015

This nicely complements my PR #4924

@e-weimann
Copy link
Contributor Author

Thanks @TerjeBr. I only guessed what the option is about.


**type**: ``array``

Used in combination with ``delivery_address``. If set, send email as normal to addresses that match with the values in the list. All addresses not matching will be sent to ``delivery_address`` as is the normal behaviour when setting a ``delivery_address``.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should add a linebreak after the first word that crosses the 72th character. This makes the source easier to read and review.

@e-weimann
Copy link
Contributor Author

@wouterj are there any other adjustments I have to do?

@wouterj
Copy link
Member

wouterj commented Feb 14, 2015

@e-weimann no, it looks good. I've now labeled it with "Finished". As soon as he has some time, @weaverryan will take a look at this PR and probably merge it.


Used in combination with ``delivery_address``. If set, send email as normal to
addresses that match with the values in the list. All addresses not matching will
be sent to ``delivery_address`` as is the normal behaviour when setting a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

behavior (we use American English for the docs) :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xabbuh Oh no! american english 😬 Well, okay... fixed it...

Used in combination with ``delivery_address``. If set, send email as normal to
addresses that match with the values in the list. All addresses not matching will
be sent to ``delivery_address`` as is the normal behavior when setting a
``delivery_address``.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should include a link to the now-merged new section about this: http://symfony.com/doc/current/cookbook/email/dev_environment.html#sending-to-a-specified-address-but-with-exceptions

And with this link, we can make the description here ever shorter - perhaps just:

Used in combination with ``deliver_address``. If set, emails matching this pattern will
be delivered like normal, instead of being sent to `delivery_address`. For details, see
LINK...

We'll need to add a new anchor link above that section (e.g. .. _cookbook-emails-delivery_whitelist) so we can link to it.

Thanks!

@xabbuh
Copy link
Member

xabbuh commented Mar 2, 2015

@e-weimann Is there a specific reason you closed your pull request?

@e-weimann
Copy link
Contributor Author

@xabbuh yes, it does not appear that the pull request will get merged, so i closed it.

@TerjeBr
Copy link
Contributor

TerjeBr commented Mar 2, 2015

@e-weimann , if you had just done the change @weaverryan asked for, I am sure it would have been merged. But that is ok, I have now created #5056 to be instead of this one.

@wouterj
Copy link
Member

wouterj commented Mar 2, 2015

My apologies (and the rest of the doc team) for giving you this feeling, @e-weimann. We are always trying to give contributors a good experience, as the contributors make a documentation great. For instance, you documented a setting in this PR that I didn't even know before this PR.

However, we are quite a small team and the doc activity has increased to a very active state (2 years ago, there were about 20 open PRs and 1 new PR in a couple of days, nowadays we get a minimum of around 3 new PRs each day and have a list of 100+ open PRs). Meanwhile, the main doc merger (@weaverryan) has moved to a new house recently. I think you can see why we are behind the facts at the moment.

I hope we don't give you or other contributors a bad experience in the future. If you have suggestions to improve the experience, please tell us.

For now, @TerjeBr applied the comment and created a new PR (Thanks @TerjeBr!). However, it's a bit sad that we can't give you the attribution for these changes.

weaverryan added a commit that referenced this pull request Mar 14, 2015
…eBr)

This PR was merged into the 2.3 branch.

Discussion
----------

  [Reference] Add missing option delivery_whitelist

Q | A
---|---
Doc fix? |	yes
New docs? | no
Applies to | SwiftMailerBundle 2.1
Fixed tickets | #4925

Commits
-------

03279dc Added double backticks. And also a full stop.
2aab241 added link target/label in the cookbook
ce8719d Changed link into ref
07855f6 Removed + from copied diff
0195619   [Reference] Add missing option delivery_whitelist
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants