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

CRM-21077 Add warning when CIVICRM_MAIL_LOG is set and you are testing outbound… #10870

Merged
merged 1 commit into from
Aug 18, 2017

Conversation

mattwire
Copy link
Contributor

… mail settings

Overview

When the environment variable CIVICRM_MAIL_LOG is set CiviCRM won't try to send mail via any of the methods, but it will return success indicating that it did send mail. Normally that's fine, except when you forget you've got that configured (or didn't realise - buildkit does it by default). With this patch, when testing outbound email settings you will get a warning message if CIVICRM_MAIL_LOG is set so you don't scratch your head for ages wondering why email is not being sent out!

Copy link
Contributor

@seamuslee001 seamuslee001 left a comment

Choose a reason for hiding this comment

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

This looks good to me @eileenmcnaughton

@totten
Copy link
Member

totten commented Aug 18, 2017

@seamuslee001 What does "looks good" mean? Does that mean you tried it out and found that the PR works as expected -- or does it mean that (upon skimming) the code seems like a plausible rendition of the concept?

@seamuslee001
Copy link
Contributor

It means to be upon skimming the code it looks sensible, strings make sense to me but i haven't yet tested it

@seamuslee001
Copy link
Contributor

I have now tested this on a buildkit install and it works as expected

@totten
Copy link
Member

totten commented Aug 18, 2017

Brilliant. :) Thank you, @seamuslee001 @mattwire.

@totten totten merged commit 9c99efa into civicrm:master Aug 18, 2017
@eileenmcnaughton
Copy link
Contributor

makes sense. I think there is a status check that covers this too?

@mattwire mattwire deleted the CRM-21077_outbound_mail_warning branch August 28, 2017 12:48
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.

4 participants