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

Hardcoded location of extra php settings for sendmail #26

Open
andriyun opened this issue Feb 27, 2017 · 4 comments
Open

Hardcoded location of extra php settings for sendmail #26

andriyun opened this issue Feb 27, 2017 · 4 comments

Comments

@andriyun
Copy link
Member

Here https://github.com/skilld-labs/skilld-docker-container/blob/master/src/docker/docker-compose.override.yml.default#L8
We are adding extra php settings for sendmail

I've faced whit issue when this files don't used whe we using another php version.
Reason: another directory for php ini files

@andriyun
Copy link
Member Author

In my case there should be /etc/php7.1/conf.d/90-mail.ini

@andypost
Copy link
Contributor

Confirmed

@pabloguerino
Copy link
Contributor

pabloguerino commented Mar 22, 2017

I suggest that we solve this by changing the .env variables a bit.

We may use only PHP_VERSION and indicate only the version number and nothing else, so you may end up picking one of this:

PHP_VERSION=5.6
PHP_VERSION=7
PHP_VERSION=7.1

then based on PHP_VERSION variable, we can generate:

PHP_IMAGE=$(echo php:$(echo $PHP_VERSION)-fpm)
PHP_CONFIG=$(echo /etc/$(echo $PHP_VERSION))

Note: this is not tested yet, so probably a copy/paste will fail, but we definitely can do this if you guys agree, so will wait for some confirmation and make a PR out of this

@pabloguerino
Copy link
Contributor

Created PR #31


Some things I noted that we can fix,

skilld-docker-container php_vars ▸ docker-compose down
WARNING: The PHP_IMAGE variable is not set. Defaulting to a blank string.
WARNING: The PHP_CONFIG variable is not set. Defaulting to a blank string.

We can add a Makefile task to solve this (make down? make stop?)

Update skilldlabs/docker-php

The naming used for the docker containers is not the optional iif we follow this path, for example for this to work on all versions/containers:

  • image: 7-fpm should remain as it is
  • image: 71-fpm should be renamed to 7.1-fpm

Source: https://github.com/skilld-labs/docker-php/blob/master/Makefile#L2

Or we can avoid to change the names and instead replace the dots inside the PR

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

No branches or pull requests

3 participants