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

Add docker-compose for controlled local development #1868

Closed
wants to merge 1 commit into from

Conversation

olvlvl
Copy link
Contributor

@olvlvl olvlvl commented Sep 28, 2018

Q A
Type feature
BC Break no

Hi,

This PR adds Docker support to help with local development.

Run make test-container to create the container and its MongoDB dependency. A terminal is opened once the container is ready, run make test to run the test suite. When you're done with the tests just enter exit and the container will be destroyed and it's dependency shutdown.

The container image is built from the official PHP 7.2 image, and includes the MongoDB driver, the bcmath extension, and Composer (to install packages from within the container).

I also added ext-bcmath to composer.json since the extension is required by the AlnumGenerator class, but I didn't indicate a version because I didn't know which one to pick.

I hope this helps.

@alcaeus
Copy link
Member

alcaeus commented Oct 5, 2018

After internal discussion, we've decided not to merge this. None of us use docker for our development environments, so maintaining the infrastructure would be an additional chore with no extra gain.

As for the bcmath extension, while technically required for AlnumGenerator, we've intentionally left it out since it's only required if you actively specify the generator in the mapping. Instead of a hard requirement, this should be checked by the class itself, with the extension put into a suggest section in composer.json to explain where it's used. Please create a separate pull request for that if you're interested in making the change.

@alcaeus alcaeus closed this Oct 5, 2018
@alcaeus alcaeus self-assigned this Oct 5, 2018
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.

None yet

2 participants