Skip to content

[XML2PHP] ReviewBundle#18771

Merged
TheMilek merged 1 commit intoSylius:2.3from
crydotsnake:xml2php/review
Feb 4, 2026
Merged

[XML2PHP] ReviewBundle#18771
TheMilek merged 1 commit intoSylius:2.3from
crydotsnake:xml2php/review

Conversation

@crydotsnake
Copy link
Copy Markdown
Member

@crydotsnake crydotsnake commented Jan 31, 2026

Q A
Branch? 2.3
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets partially #18490
License MIT

Summary by CodeRabbit

  • Refactor
    • Modernized internal configuration format for the Review bundle to improve maintainability and consistency with framework standards.

@crydotsnake crydotsnake requested review from a team as code owners January 31, 2026 11:54
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 31, 2026

📝 Walkthrough

Walkthrough

The Review Bundle's service configurations are migrated from XML to PHP format. The dependency injection extension loader switches from XmlFileLoader to PhpFileLoader, and equivalent service definitions for the average rating calculator and Doctrine ORM event subscriber are relocated from XML to corresponding PHP configuration files.

Changes

Cohort / File(s) Summary
Extension Configuration Loading
src/Sylius/Bundle/ReviewBundle/DependencyInjection/SyliusReviewExtension.php
Updated loader class from XmlFileLoader to PhpFileLoader and changed service configuration file paths from .xml to .php for both services and integrations directories.
Services Configuration
src/Sylius/Bundle/ReviewBundle/Resources/config/services.php, src/Sylius/Bundle/ReviewBundle/Resources/config/services.xml
Migrated sylius.calculator.average_rating service registration from XML to PHP configuration format; XML file content replaced with equivalent PHP DI setup.
Doctrine ORM Integration
src/Sylius/Bundle/ReviewBundle/Resources/config/integrations/doctrine/orm.php, src/Sylius/Bundle/ReviewBundle/Resources/config/integrations/doctrine/orm.xml
Migrated sylius.doctrine.orm.event_subscriber.load_metadata.review service registration from XML to PHP; registers LoadMetadataSubscriber with doctrine.event_subscriber tag and %sylius.review.subjects% parameter injection.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested labels

Maintenance

Suggested reviewers

  • GSadee
  • Rafikooo

Poem

🐰 From XML tags we hop to PHP's grace,
Services now configured in a cleaner place,
Config files migrate without a trace,
The ReviewBundle runs at a quicker pace! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly references the main change: converting XML configuration files to PHP format in the ReviewBundle, which is the primary focus across all modified files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 31, 2026

❌ Preview Environment deleted from Bunnyshell

Available commands:

  • 🚀 /bns:deploy to redeploy the environment

Comment on lines +20 to +27
$parameters = $container->parameters();

$services->defaults()
->public();

$services->set('sylius.doctrine.orm.event_subscriber.load_metadata.review', LoadMetadataSubscriber::class)
->args(['%sylius.review.subjects%'])
->tag('doctrine.event_subscriber');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$parameters = $container->parameters();
$services->defaults()
->public();
$services->set('sylius.doctrine.orm.event_subscriber.load_metadata.review', LoadMetadataSubscriber::class)
->args(['%sylius.review.subjects%'])
->tag('doctrine.event_subscriber');
$services->set('sylius.doctrine.orm.event_subscriber.load_metadata.review', LoadMetadataSubscriber::class)
->args(['%sylius.review.subjects%'])
->public()
->tag('doctrine.event_subscriber');

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In
`@src/Sylius/Bundle/ReviewBundle/Resources/config/integrations/doctrine/orm.php`:
- Around line 18-27: The closure returned at "return static function
(ContainerConfigurator $container)" should declare a void return type to follow
the type-declaration guideline; update the anonymous function signature to
"static function (ContainerConfigurator $container): void" so the configurator
closure (the ContainerConfigurator-typed anonymous function that sets up
$services and registers
'sylius.doctrine.orm.event_subscriber.load_metadata.review') explicitly returns
void.

In `@src/Sylius/Bundle/ReviewBundle/Resources/config/services.php`:
- Around line 18-22: The closure returned from the services configurator is
missing an explicit return type; update the anonymous function signature (the
static function taking ContainerConfigurator $container in services.php that
registers 'sylius.calculator.average_rating' => AverageRatingCalculator::class)
to declare a void return type (i.e., static function (ContainerConfigurator
$container): void) so it adheres to the project's explicit return-type
guidelines.

Comment on lines +18 to +27
return static function (ContainerConfigurator $container) {
$services = $container->services();

$services->defaults()
->public();

$services->set('sylius.doctrine.orm.event_subscriber.load_metadata.review', LoadMetadataSubscriber::class)
->args(['%sylius.review.subjects%'])
->tag('doctrine.event_subscriber');
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add a void return type to the configurator closure.

This keeps the config file compliant with the type declaration guideline.

✅ Suggested fix
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return static function (ContainerConfigurator $container) {
$services = $container->services();
$services->defaults()
->public();
$services->set('sylius.doctrine.orm.event_subscriber.load_metadata.review', LoadMetadataSubscriber::class)
->args(['%sylius.review.subjects%'])
->tag('doctrine.event_subscriber');
};
return static function (ContainerConfigurator $container): void {
$services = $container->services();
$services->defaults()
->public();
$services->set('sylius.doctrine.orm.event_subscriber.load_metadata.review', LoadMetadataSubscriber::class)
->args(['%sylius.review.subjects%'])
->tag('doctrine.event_subscriber');
};
🤖 Prompt for AI Agents
In
`@src/Sylius/Bundle/ReviewBundle/Resources/config/integrations/doctrine/orm.php`
around lines 18 - 27, The closure returned at "return static function
(ContainerConfigurator $container)" should declare a void return type to follow
the type-declaration guideline; update the anonymous function signature to
"static function (ContainerConfigurator $container): void" so the configurator
closure (the ContainerConfigurator-typed anonymous function that sets up
$services and registers
'sylius.doctrine.orm.event_subscriber.load_metadata.review') explicitly returns
void.

Comment on lines +18 to +22
return static function (ContainerConfigurator $container) {
$services = $container->services();

$services->set('sylius.calculator.average_rating', AverageRatingCalculator::class);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add a void return type to the configurator closure.

Guidelines require explicit return types, and the closure currently omits it.

✅ Suggested fix
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {
🤖 Prompt for AI Agents
In `@src/Sylius/Bundle/ReviewBundle/Resources/config/services.php` around lines 18
- 22, The closure returned from the services configurator is missing an explicit
return type; update the anonymous function signature (the static function taking
ContainerConfigurator $container in services.php that registers
'sylius.calculator.average_rating' => AverageRatingCalculator::class) to declare
a void return type (i.e., static function (ContainerConfigurator $container):
void) so it adheres to the project's explicit return-type guidelines.

@TheMilek TheMilek merged commit 78cef35 into Sylius:2.3 Feb 4, 2026
35 of 36 checks passed
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