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

dev: add development container to docker-compose.yaml #62

Merged
merged 2 commits into from
Mar 8, 2024

Conversation

BacLuc
Copy link

@BacLuc BacLuc commented Feb 28, 2024

To simplify contribution.
To install the dependencies:
docker compose run --rm php composer install --prefer-dist

To run the tests:
docker compose run --rm php vendor/bin/phpunit --verbose

The profile in the docker-compose.yaml file is used that the php service does not start with the mysql service, and it is only used for one shot commands, not as
a daemon.

To simplify contribution.
To install the dependencies:
docker compose run --rm php composer install --prefer-dist

To run the tests:
docker compose run --rm php vendor/bin/phpunit --verbose

The profile in the docker-compose.yaml file is used that
the php service does not start with the mysql service,
and it is only used for one shot commands, not as
a daemon.
@BacLuc
Copy link
Author

BacLuc commented Feb 28, 2024

Would you be open to such i change?
Should i add documentation somewhere?

Copy link
Owner

@theofidry theofidry left a comment

Choose a reason for hiding this comment

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

I think it's a great idea, but I would then push it a step further:

  • There is also MongoDB
  • I would adjust the Makefile and related scripts to make use of the docker image
  • Might be worth using it in the CI as well to avoid having a difference between a local run and a CI run?

It's by no means a small contribution, but that would definitely make a lot of things a lot easier 😊

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@BacLuc
Copy link
Author

BacLuc commented Feb 29, 2024

  • There is also MongoDB

Did i understand correctly that i should add the mongodb driver, not a mongodb service to the docker compose?
I ran the tests and they all passed without a mongodb driver or service.

I added a mongodb driver for now, but did not test it.

  • I would adjust the Makefile and related scripts to make use of the docker image

Could you point me to the Makefile i need to update?

  • Might be worth using it in the CI as well to avoid having a difference between a local run and a CI run?

It makes the code dry and everything is updated in one step.
But i also made the experience that it is slower to build / run the tests in a container in github actions,
than to run them in the action itself.
And if you let renovate update it, it updates all the places for you if configured correctly.
There may be problems with different environments, but we only had 1 in 3 years: ecamp/ecamp3#3457 (comment)

@theofidry
Copy link
Owner

@BacLuc apologies for the late review.

Looks like I mixed up this PR with https://github.com/theofidry/AliceDataFixtures, so forget about the last comment about MongoDB and the Makefile 😓.

Thanks for the work!

@theofidry theofidry merged commit ec2877a into theofidry:master Mar 8, 2024
3 of 5 checks passed
@BacLuc BacLuc deleted the add-dev-container branch March 8, 2024 19:25
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.

2 participants