Skip to content

Conversation

@Chris53897
Copy link
Contributor

@Chris53897 Chris53897 commented Dec 22, 2025

#378

Summary by CodeRabbit

  • New Features

    • Added support for Symfony 8.0 and compatibility across PHP 8.0–8.3.
  • Chores

    • Updated Symfony component version constraints to include version 8.0.
    • Added explicit return type declarations to methods.
    • Enhanced test infrastructure with dynamic version-based configuration selection.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025

Walkthrough

The PR adds Symfony 8.0 component support across the codebase by updating dependency constraints in composer.json, extending CI matrix coverage for Symfony 8.0 with PHP versions 8.0–8.3, adding explicit void return types to command and validator methods, and implementing version-aware test configurations that dynamically select framework files.

Changes

Cohort / File(s) Change Summary
CI and Dependency Updates
.github/workflows/ci.yml, composer.json
Added Symfony 8.0.\* to test matrix; introduced exclusion pairs for incompatible PHP/Symfony combinations; broadened component version constraints from ^5.4 || ^6.4 || ^7.0 to ^5.4 || ^6.4 || ^7.0 || ^8.0 across multiple packages.
Method Signature Enhancements
src/Command/GeocodeCommand.php, src/Validator/Constraint/AddressValidator.php
Added explicit : void return type annotations to configure() and validate() methods, removing corresponding docblock return declarations.
Dynamic Test Configuration
tests/Functional/BundleInitializationTest.php, tests/Functional/GeocoderListenerTest.php, tests/Functional/PluginInteractionTest.php, tests/Functional/ProviderFactoryTest.php
Replaced hardcoded framework_sf6.yml with dynamic framework_sf{MAJOR_VERSION}.yml path construction in test setup methods, preserving existing version conditionals (VERSION_ID >= 60000).
New Framework Configuration Files
tests/Functional/config/framework_sf7.yml, tests/Functional/config/framework_sf8.yml
Added version-specific Symfony framework configurations with handle_all_throwables: true and php_errors.log: true for Symfony 7+ compatibility.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Method signature additions follow a consistent, straightforward pattern applied to two files
  • Dependency version constraint updates are mechanical and uniform
  • Test configuration changes replace hardcoded strings with simple templated variables across four files
  • New configuration files are simple YAML additions without complex conditional logic

Possibly related PRs

Suggested reviewers

  • norkunas

Poem

🐰 Hop, skip, and a jump to version eight,
Dynamic configs make testing first-rate,
Return types explicit, we're standing so tall,
Symfony's new features now answer the call!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 'feat: allow symfony 8' directly and clearly describes the main objective of the PR: adding Symfony 8 support to the project.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ceaf27 and 9f2452a.

📒 Files selected for processing (10)
  • .github/workflows/ci.yml
  • composer.json
  • src/Command/GeocodeCommand.php
  • src/Validator/Constraint/AddressValidator.php
  • tests/Functional/BundleInitializationTest.php
  • tests/Functional/GeocoderListenerTest.php
  • tests/Functional/PluginInteractionTest.php
  • tests/Functional/ProviderFactoryTest.php
  • tests/Functional/config/framework_sf7.yml
  • tests/Functional/config/framework_sf8.yml
🧰 Additional context used
🧬 Code graph analysis (1)
src/Validator/Constraint/AddressValidator.php (1)
tests/Validator/Constraint/AddressValidatorTest.php (2)
  • testNullIsValid (49-54)
  • testEmptyStringIsValid (56-61)
🔇 Additional comments (11)
tests/Functional/config/framework_sf7.yml (1)

1-5: LGTM!

The Symfony 7 framework configuration is appropriate, disabling annotations (in favor of attributes) and enabling comprehensive error handling for tests.

src/Validator/Constraint/AddressValidator.php (1)

41-41: LGTM!

The explicit void return type correctly implements the ConstraintValidator interface contract and improves type safety.

src/Command/GeocodeCommand.php (1)

39-39: LGTM!

The explicit void return type correctly implements the Command::configure() method signature and improves type safety.

composer.json (1)

20-22: LGTM!

Symfony version constraints are correctly and consistently extended to include ^8.0 across all required and development dependencies.

Also applies to: 63-69

tests/Functional/config/framework_sf8.yml (1)

1-4: LGTM!

The Symfony 8 framework configuration appropriately enables comprehensive error handling. The absence of the annotations setting compared to framework_sf7.yml is expected, as Symfony 8 has fully transitioned to attributes.

tests/Functional/ProviderFactoryTest.php (1)

81-81: LGTM!

The dynamic framework configuration selection using $kernel::MAJOR_VERSION enables proper testing across Symfony 6, 7, and 8, and aligns with the newly added version-specific config files.

tests/Functional/PluginInteractionTest.php (2)

62-62: LGTM!

The dynamic framework configuration selection ensures proper test execution across different Symfony versions.


88-88: LGTM!

Consistent use of dynamic framework configuration selection throughout the test.

.github/workflows/ci.yml (1)

73-80: Incorrect analysis of Symfony 8.0 PHP requirements.

Symfony 8.0 requires PHP 8.4.0 or higher, not PHP 8.2 as stated in the review. The exclusions for PHP 8.2 and 8.3 with Symfony 8.0.* are correct and should not be removed. Only PHP 8.4+ should be tested with Symfony 8.0.*.

Likely an incorrect or invalid review comment.

tests/Functional/BundleInitializationTest.php (1)

57-57: LGTM! Dynamic framework configuration selection.

The change from hardcoded framework_sf6.yml to dynamic framework_sf{MAJOR_VERSION}.yml is implemented consistently across all test methods, matching the pattern in GeocoderListenerTest.php. This enables proper version-aware testing for Symfony 6, 7, and 8.

Also applies to: 75-75, 95-95, 117-117, 147-147, 169-169

tests/Functional/GeocoderListenerTest.php (1)

111-111: LGTM! Dynamic framework configuration selection.

The change from hardcoded framework_sf6.yml to dynamic framework_sf{MAJOR_VERSION}.yml correctly enables version-aware test configuration for Symfony 6, 7, and 8. The implementation is consistent across all test methods, and all required configuration files (framework_sf6.yml, framework_sf7.yml, framework_sf8.yml) exist in tests/Functional/config/.


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.

@norkunas norkunas merged commit 5f129d6 into geocoder-php:master Dec 22, 2025
18 checks passed
@norkunas
Copy link
Member

Thank you @Chris53897

@Chris53897 Chris53897 deleted the feature/symfony-8 branch December 22, 2025 19:26
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.

3 participants