Skip to content

[5.0] Set mail config values in .env file. #3269

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 23, 2015
Merged

[5.0] Set mail config values in .env file. #3269

merged 1 commit into from
Feb 23, 2015

Conversation

martinbean
Copy link
Contributor

Make the mail configuration values look in .env file by default.

crynobone
crynobone reviewed Feb 17, 2015
View reviewed changes
.env.example
@@ -8,4 +8,5 @@ DB_USERNAME=homestead
DB_PASSWORD=secret

CACHE_DRIVER=file
MAIL_DRIVER=file
Copy link
Member

Choose a reason for hiding this comment

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

file mail driver?

Copy link
Member

Choose a reason for hiding this comment

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

lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@crynobone @GrahamCampbell Yeah, that’s a typo 😊

Fixed! Thanks for pointing that out.

@laurencei
Copy link
Contributor

I like this PR as I find myself making this exact change on all my current L5 projects so far.

But I think the naming convention is wrong.

Instead of names like SMTP_USERNAME - it should be MAIL_USERNAME - because you might not be using an SMTP driver. And SMTP_HOST should be MAIL_HOST and so on and so on etc

The only other thought - since you do not have a default username+password - these should also probably be included in the default .env file since you are expected to set them?

@martinbean
Copy link
Contributor Author

@theshiftexchange Thanks for the feedback. I’m named them as such as the comment blocks above each variable explicitly mention they’re the SMTP host/port/username/password configuration values.

Happy to rename them if these credentials are used in a non-SMTP instances, but can’t think of any of those instances off the top of my head.

@laurencei
Copy link
Contributor

No problems - its just a thought.

What about since you do not have a default username+password in the config env() - these should also probably be included in the default .env file since you are expected to set them? i.e. like is already done for the DB settings.

taylorotwell added a commit that referenced this pull request Feb 23, 2015
@taylorotwell
Merge pull request #3269 from martinbean/set-mail-defaults
cc09679 
[5.0] Set mail config values in .env file.
@taylorotwell taylorotwell merged commit cc09679 into laravel:master Feb 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@martinbean @laurencei @crynobone @GrahamCampbell @taylorotwell