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

Fixes issue where .env file is invalid #61

Merged
merged 1 commit into from
Apr 19, 2016
Merged

Conversation

adamlc
Copy link
Contributor

@adamlc adamlc commented Apr 18, 2016

This fixes #60 where the .env file is created without quotes

This fixes cachethq#60 where the .env file is created without quotes
@jbrooksuk
Copy link
Member

Most of these settings should never need to be double-quoted, but if it works then it's only going to prevent issues further down the line 👍

@adamlc
Copy link
Contributor Author

adamlc commented Apr 18, 2016

Ill be honest I didn't think all of them would need to be, but I guess it won't hurt though 😄

@djdefi
Copy link
Contributor

djdefi commented Apr 18, 2016

Interesting find.

Dotenv values containing spaces must be surrounded by quotes.

Out of curiosity, which attribute were you using a space in?

@jbrooksuk
Copy link
Member

Usually it's MAIL_NAME that causes an issue.

@adamlc
Copy link
Contributor Author

adamlc commented Apr 19, 2016

Yeah it was just MAIL_NAME actually. I guess thats probably the only one that will need double quotes. I'm not sure if theres a better way of doing this in the entrypoint script?

@jbrooksuk
Copy link
Member

This is probably the simplest place.

@adamlc
Copy link
Contributor Author

adamlc commented Apr 19, 2016

Shall I modify the PR to only change the MAIL_NAME?

@jbrooksuk
Copy link
Member

It's possible that MAIL_PASSWORD could have a space.

@adamlc
Copy link
Contributor Author

adamlc commented Apr 19, 2016

Well does it hurt leaving them all double quoted?

@jbrooksuk
Copy link
Member

Not that I know of.

@djdefi djdefi merged commit 10c2709 into cachethq:master Apr 19, 2016
@mattallty
Copy link
Contributor

It hurts: see #151

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.

Container won't start
4 participants