Clarify "config.action_mailer.raise_delivery_errors = true" #7851

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

Alamoz commented Oct 5, 2012

...ents/production.rb.tt

Setting config.action_mailer.raise_delivery_errors = true only raises email delivery errors if the email server is configured a certain way. You will have to consult the documentation for your particular email server. Refer to discussion at #7473 (comment)

Adrien Lamothe Update railties/lib/rails/generators/rails/app/templates/config/envir…
…onments/production.rb.tt

Setting config.action_mailer.raise_delivery_errors = true only raises email delivery errors if the email server is configured a certain way. You will have to consult the documentation for your particular email server.
ea6b309
Member

steveklabnik commented Oct 5, 2012

Don't forget about docrails.

Member

vijaydev commented Oct 5, 2012

Usually, I like such changes to be made here as PRs. That is, changes in templates we use to generate the app - more eyeballs the better.

@vijaydev vijaydev and 1 other commented on an outdated diff Oct 5, 2012

...ls/app/templates/config/environments/production.rb.tt
@@ -56,6 +56,8 @@
<%- end -%>
# Disable delivery errors, bad email addresses will be ignored.
+ # If set to true, delivery errors will not be raised unless your
@vijaydev

vijaydev Oct 5, 2012

Member

The double negative makes this hard to parse. We can have it in the positive sense like in the commit message.

@vijaydev

vijaydev Oct 5, 2012

Member

On second thoughts, I find this addition problematic.

As someone reading this, I don't know what does proper configuration of the email server mean. Is it the basic config to enable the server to send emails OR does rails require some special config in the email server to be set to raise delivery errors other than this config option here?

If we go about clarifying that in detail, we may have a small paragraph here, which is not the purpose here and which should go into documentation (guides or api).

@Alamoz

Alamoz Oct 6, 2012

Actually, change "properly" to "immediately deliver email" and that explicitly defines it.

Alamoz commented Oct 5, 2012

We can change the text to clarify it, but I think it would be really good to communicate that only setting config.action_mailer.raise_delivery_errors = true is usually not enough to insure delivery errors are raised. This will save people a fair amount of time they will have to spend figuring out why no delivery errors are raised.

I'll clean up the verbiage and see if you guys find it acceptable. I'll also gladly add something to docrails.

Alamoz commented Oct 5, 2012

I think this is succinct and covers everything:

Ignore bad email addresses and do not raise email delivery errors.

If set to true, delivery errors will be raised if the external email server is configured properly.

config.action_mailer.raise_delivery_errors = false

Alamoz commented Oct 5, 2012

OK, just added the clarification to docrails, see https://github.com/lifo/docrails/pull/117

Member

steveklabnik commented Oct 6, 2012

@vijaydev :

That is, changes in templates we use to generate the app - more eyeballs the better.

ahh, that makes sense. I won't consider template comments the same as code comments now.

Since this was added to docrails, I'm closing the PR. Sorry about the back and forth.

Alamoz commented Oct 6, 2012

I'm unclear on this. Is the comment change made in the docrails template going to make it into the rails environments/production.rb?

Member

steveklabnik commented Oct 6, 2012

Oh. Sorry. So, you shouldn't open a pull on docrails. Just push to it. That's the whole point. It has open commit access. Every so often, it gets merged back into core.

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