Feature/update dependencies - #34
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates dependency management and project tooling to align the repo with newer Symfony/Doctrine/API Platform versions, and streamlines developer commands through the Makefile.
Changes:
- Switch
update-deps.shto usemake down/build/api/pwainstead of directdocker composeinvocations. - Add a
pwaMakefile target to run commands in the PWA container. - Upgrade PHP dependencies (Symfony to
^8.1, Doctrine/API Platform, etc.) and adjust Doctrine config/reference types accordingly.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| update-deps.sh | Uses Make targets for dependency update workflow; updates test command hint. |
| Makefile | Adds pwa target for running commands in the PWA container. |
| api/config/reference.php | Updates Symfony config reference Psalm types to match newer component/bundle versions. |
| api/config/packages/doctrine.yaml | Removes legacy ORM proxy settings; keeps native lazy objects config. |
| api/composer.json | Bumps Symfony/Doctrine/API Platform and related dependencies to newer major/minor versions. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
update-deps.sh:13
make api "/bin/sh -c ..."andmake pwa ...pass the command as an extra Make goal, not as arguments to theapi/pwatargets. This will try to build a target named/bin/sh -c '...'and will fail with “No rule to make target …”. Pass the container command via a variable (or switch back todocker compose run).
make api "/bin/sh -c 'composer update; composer outdated'"
make pwa "/bin/sh -c 'pnpm install; pnpm update; pnpm outdated'"
update-deps.sh:21
- This
echoline’s quoting is currently assembling the command by concatenating quoted segments, so the resulting output drops the quotes around the/bin/sh -c '...'argument. That makes the printed command misleading (and copy/paste likely won’t do what it intends). Use double quotes for the outer string and escape the literal backticks so the printed command includes the inner single quotes correctly.
echo 'Run `docker compose exec api /bin/sh -c ''bin/console -e test doctrine:database:create ; bin/console -e test doctrine:migrations:migrate --no-interaction ; bin/phpunit ; bin/console -e test doctrine:schema:validate''` to check that the tests are green.'
…is now builtin support in Symfony
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Code Coverage SummaryDiff against developResults for commit: fba3e27 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
update-deps.sh:13
make api "/bin/sh -c ..."/make pwa ...will be interpreted bymakeas additional targets, causing a "No rule to make target" error. If you want to pass a command to theapi/pwatargets, pass it via theArgumentsvariable (which also avoids needing Makefile tricks to ignore extra goals).
make api Arguments="/bin/sh -c 'composer update; composer outdated'"
make pwa Arguments="/bin/sh -c 'pnpm install; pnpm update; pnpm outdated'"
api/composer.json:49
- Composer version constraints typically omit the leading
v. Using^v2.9.1is inconsistent with the rest of this file and can be confusing; prefer^2.9.1.
"php-coveralls/php-coveralls": "^v2.9.1",
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
update-deps.sh:5
make downonly runsdocker compose down(no-v), so the script no longer removes volumes even though it previously did (down -v). For dependency updates/migrations this can leave stale Postgres/RabbitMQ state and produce misleading results; usemake clean(down + volumes + orphans) to keep the script’s “clean slate” intent.
# Remove all running containers
make down
#docker compose down -v
Makefile:64
- The
pwatarget (and similarlyapi/worker) forces-it, which breaks non-interactive usage (e.g. runningupdate-deps.shfrom CI or any shell without a TTY) with errors like “the input device is not a TTY”. Prefer disabling TTY allocation by default (-T) so automation works reliably; developers can still run interactive commands directly withdocker compose run -it ...when needed.
.PHONY: pwa
pwa:
$(DOCKER_COMPOSE_PREFIX) docker compose run --entrypoint="" --rm -it pwa $(Arguments)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Types of changes