fix: add check on models and migrations to ensure migrations are included in PR#546
Open
rinkp wants to merge 4 commits into
Open
fix: add check on models and migrations to ensure migrations are included in PR#546rinkp wants to merge 4 commits into
rinkp wants to merge 4 commits into
Conversation
rinkp
force-pushed
the
fix/migrations
branch
2 times, most recently
from
407f837 to
803f31e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent missing Doctrine migrations by (1) adjusting how migrations are generated/cleaned up locally and (2) adding a GitHub Actions workflow that regenerates migrations in CI and fails the PR if new migration files would be produced. It also updates the shared VersionTrait mapping to introduce a default value for the version column.
Changes:
- Update
VersionTraitto set a DB-level default (1000) for the optimistic-locking version column. - Extend
make migration-diffto post-process generated migration files and delete “empty” migrations. - Add a new GitHub Actions workflow to detect missing migrations on pull requests that touch models/migrations.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
module/Database/src/Model/Trait/VersionTrait.php |
Adds a default to the Doctrine #[Column] mapping for the version field. |
Makefile |
Adds migration post-processing (sed cleanup + deleting empty migrations) to migration-diff. |
.github/workflows/ensure-migrations.yml |
New CI workflow to regenerate migrations and fail if changes appear under migrations/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rinkp
changed the title
Fix open migration issue
5 tasks
Member
|
This remark also comes from: #545 (comment). So I think we should add changes to the existing workflow to this PR as well. |
Makes the check lightweight by only starting the web and Postgres containers. Custom template for the migrations such that we can more easily test migrations instead of relying on `sed`. Combines the up-to-date check with the existing migration and seed workflow into a single workflow on the same web and Postgres stack. Splits `seed` so it only seeds the core databases (fixtures and report), as Mailman and Listmonk are sync-based and not needed to test migrations or seeders.
Member
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.
Description
With this PR, three migration-related issues are resolved:
VerstionTraitofConfigItemis resolvedTypes of changes