Add support for doctrine bundle v3 #400
Conversation
|
Warning Rate limit exceeded@norkunas has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (10)
WalkthroughAdded Doctrine Bundle v3 support while maintaining v2 compatibility. Updated composer.json constraint to allow both versions. Refactored test configuration to detect and load version-specific configs dynamically. Expanded CI matrix to test multiple PHP and Symfony versions with different dependency constraints. Changes
Sequence Diagram(s)sequenceDiagram
participant Kernel as Kernel.php
participant ClassLoader as PHP Runtime
participant ConfigLoader as Config Loader
Kernel->>ClassLoader: Check if BlacklistSchemaAssetFilter exists
ClassLoader-->>Kernel: class_exists result
alt Doctrine v3 Active (class does not exist)
Kernel->>ConfigLoader: Load framework.yaml
Kernel->>ConfigLoader: Load config.yaml
else Doctrine v2 (class exists)
Kernel->>ConfigLoader: Load framework.yaml
alt PHP 8+ with LegacyReflectionFields
Kernel->>ConfigLoader: Load config_doctrine_v2.yaml
else PHP 8+ or PHP 7
Kernel->>ConfigLoader: Load config_old_proxy.yaml or doctrine_php7.yaml
end
end
ConfigLoader-->>Kernel: Configuration loaded
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
norkunas
force-pushed
the
doctrinebundlev3
branch
2 times, most recently
from
0a4e91a to
9c4efad
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/tests.yml(2 hunks)composer.json(1 hunks)tests/Kernel.php(2 hunks)tests/config/config.yaml(0 hunks)tests/config/config_doctrine_v2.yaml(1 hunks)tests/config/config_old_proxy.yaml(0 hunks)tests/config/doctrine_php7.yaml(0 hunks)tests/config/framework.yaml(1 hunks)
💤 Files with no reviewable changes (3)
- tests/config/doctrine_php7.yaml
- tests/config/config_old_proxy.yaml
- tests/config/config.yaml
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Kernel.php (1)
src/DependencyInjection/MeilisearchExtension.php (1)
load(19-64)
🪛 actionlint (1.7.8)
.github/workflows/tests.yml
101-101: property "dependencies" is not defined in object type {deps: string; php-version: number; sf-version: number}
(expression)
🔇 Additional comments (8)
tests/config/framework.yaml (1)
1-4: LGTM!Standard framework configuration for test environment. The settings are appropriate for integration tests.
tests/config/config_doctrine_v2.yaml (1)
1-22: LGTM! Doctrine v2-specific configuration is well-structured.The configuration correctly isolates v2-specific ORM options (
enable_native_lazy_objects,report_fields_where_declared) that are not available or have different semantics in v3.tests/Kernel.php (4)
11-11: LGTM! Appropriate version detection strategy.Using
BlacklistSchemaAssetFilteras a version marker is a pragmatic approach since this class was removed in DoctrineBundle v3.
33-33: LGTM! Framework config externalized correctly.Loading framework configuration from a separate file improves maintainability and aligns with the version-specific config strategy.
35-44: LGTM! Version-aware config loading is well-structured.The conditional logic correctly handles:
- DoctrineBundle v3 (loads config.yaml)
- DoctrineBundle v2 with PHP 8.4+ and LegacyReflectionFields (loads config_doctrine_v2.yaml)
- Older setups (loads config_old_proxy.yaml)
46-53: LGTM! PHP 7 compatibility handled appropriately.The PHP 7 branch correctly configures annotations and loads the PHP 7-specific Doctrine config.
composer.json (1)
23-23: The constraint is correct—no changes needed.The
^2.10 || ^3.0constraint properly handles version resolution across supported PHP versions. DoctrineBundle v2.10 requires PHP ^7.4 or ^8.0, covering all versions your package supports. DoctrineBundle v3 requires PHP ^8.4, so Composer will:
- Install v2.x on PHP 7.4-8.3 (where v3 is unavailable)
- Install v3.x on PHP 8.4+ (where both are available)
This is appropriate package design that supports multiple dependency versions across different PHP versions.
.github/workflows/tests.yml (1)
101-101: No action required—default behavior is safe and correct.The ramsey/composer-install action defaults to "locked" when dependency-versions is empty, installing locked versions equivalent to running composer install. This is the intended behavior for matrix entries without an explicit dependencies property, and no verification or code changes are needed.
Likely an incorrect or invalid review comment.
norkunas
force-pushed
the
doctrinebundlev3
branch
6 times, most recently
from
be47be4 to
2db723c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #400 +/- ##
=========================================
Coverage 88.40% 88.40%
Complexity 1 1
=========================================
Files 20 20
Lines 888 888
=========================================
Hits 785 785
Misses 103 103 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
norkunas
force-pushed
the
doctrinebundlev3
branch
4 times, most recently
from
8229d43 to
ae0b546
Compare
norkunas
force-pushed
the
doctrinebundlev3
branch
from
ae0b546 to
51f442c
Compare
1 participant
Pull Request
Related issue
Fixes #399
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Summary by CodeRabbit
Release Notes