Skip to content

PHPORM-31 Split documentation into multiple files #2610

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

Merged
merged 3 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .codacy.yml

This file was deleted.

2 changes: 0 additions & 2 deletions .styleci.yml

This file was deleted.

28 changes: 27 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,32 @@ Before submitting a pull request:
- Check the codebase to ensure that your feature doesn't already exist.
- Check the pull requests to ensure that another person hasn't already submitted the feature or fix.

## Run Tests

The full test suite requires PHP cli with mongodb extension, a running MongoDB server and a running MySQL server.
Tests requiring MySQL will be skipped if it is not running.
Duplicate the `phpunit.xml.dist` file to `phpunit.xml` and edit the environment variables to match your setup.

```bash
$ docker-compose up -d mongodb mysql
$ docker-compose run tests
```

Docker can be slow to start. You can run the command `php vendor/bin/phpunit --testdox` locally or in a docker container.

```bash
$ docker-compose run -it tests bash
# Inside the container
$ composer install
$ vendor/bin/phpunit --testdox
```

For fixing style issues, you can run the PHP Code Beautifier and Fixer:

```bash
$ php vendor/bin/phpcbf
```

## Requirements

If the project maintainer has any additional requirements, you will find them listed here.
Expand All @@ -44,7 +70,7 @@ If the project maintainer has any additional requirements, you will find them li

- **Add tests!** - Your patch won't be accepted if it doesn't have tests.

- **Document any change in behaviour** - Make sure the `README.md` and any other relevant documentation are kept up-to-date.
- **Document any change in behaviour** - Make sure the documentation is kept up-to-date.

- **Consider our release cycle** - We try to follow [SemVer v2.0.0](https://semver.org/). Randomly breaking public APIs is not an option.

Expand Down
10 changes: 4 additions & 6 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@ RUN apt-get update && \
pecl install xdebug && docker-php-ext-enable xdebug && \
docker-php-ext-install -j$(nproc) pdo_mysql zip

COPY --from=composer:2.5.8 /usr/bin/composer /usr/local/bin/composer
COPY --from=composer:2.6.2 /usr/bin/composer /usr/local/bin/composer

ENV COMPOSER_ALLOW_SUPERUSER=1

WORKDIR /code

COPY ./ ./

ENV COMPOSER_ALLOW_SUPERUSER=1

RUN composer install

CMD ["./vendor/bin/phpunit", "--testdox"]
CMD ["bash", "-c", "composer install && ./vendor/bin/phpunit --testdox"]
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, what's the advantage of moving composer install into CMD from its separate RUN? Does it mean that the built image will be without composer dependencies and they will be installed before running tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

The working directory is mounted into the container by docker-compose, so dependencies installed the docker image are ignored.
Also, if you ran composer install with PHP 8.2 locally and try to run phpunit in the container with PHP 8.1, it will fail. That's also why I add config.platform.php: 8.1 in composer.json

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks for explaining!

Loading