Skip to content

Conversation

@martin-georgiev
Copy link
Owner

@martin-georgiev martin-georgiev commented Oct 23, 2025

⚠️ These changes are scheduled for release around NYE 2025/26.

Summary by CodeRabbit

  • Chores
    • Updated minimum PHP version requirement to 8.2 across the project, officially dropping support for PHP 8.1
    • Updated continuous integration test matrix to validate the codebase against PHP 8.2, 8.3, 8.4, and 8.5
    • Enhanced code formatting and style standards with additional PHP 8 migration rules
    • Minor formatting adjustments to test infrastructure files

@coderabbitai
Copy link

coderabbitai bot commented Oct 23, 2025

Walkthrough

The PR removes PHP 8.1 from CI test matrices and updates the project's minimum PHP requirement to 8.2. The composer.json constraint changes from ^8.1 <8.6 to ^8.2 <8.6. PHP-cs-fixer configuration adds PHP 8 migration rules, and test data formatting is adjusted.

Changes

Cohort / File(s) Summary
CI Workflow Configuration
.github/workflows/integration-tests.yml, .github/workflows/unit-tests.yml
Removed PHP 8.1 from test matrices. Unit-tests also removes the corresponding matrix include entry with Doctrine ORM 2.14 and Lexer 1.2 dependencies.
Project Dependencies
composer.json
Updated minimum PHP version constraint from ^8.1 <8.6 to ^8.2 <8.6.
Code Quality Configuration
ci/php-cs-fixer/config.php
Added PHP 8 migration rule sets (@PHP8x1Migration and @PHP8x2Migration) to fixer rules.
Test Data Formatting
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php
Adjusted heredoc string literal boundaries by indenting closing END markers in test data arrays.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Homogeneous pattern across changes (PHP 8.1 removal)
  • Straightforward configuration updates with minimal risk
  • One minor test data formatting adjustment

Possibly related PRs

  • #467: Modifies PHP version constraints and CI workflows for version management.
  • #465: Adjusts CI workflows' PHP version matrices and composer constraints.
  • #425: Updates PHP version matrices in unit-tests and integration-tests workflows.

Poem

🐰 Farewell to 8.1, we bid adieu,
Eight-point-two now leads our crew,
With cs-fixer rules refined,
A stronger PHP version we find! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: dropping PHP 8.1 support, which is reflected across multiple files (workflows, composer.json, and config).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dropphp81

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.

@martin-georgiev martin-georgiev marked this pull request as draft October 23, 2025 14:46
@coveralls
Copy link

coveralls commented Oct 23, 2025

Coverage Status

coverage: 95.725%. remained the same
when pulling 2e26fab on dropphp81
into 75a1d63 on main.

Copy link

@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: 0

🧹 Nitpick comments (1)
.github/workflows/unit-tests.yml (1)

86-96: Remove unused doctrine-lexer 1.2 installation logic.

Lines 88-89 reference doctrine-lexer "1.2", which is no longer present in the matrix (line 47 shows ['2.1', '3.0', 'latest']). This conditional block is now dead code and should be removed.

Apply this diff:

       - name: Install Doctrine Lexer dependency
         run: |
-          if [ "${{ matrix.doctrine-lexer }}" = "1.2" ]; then
-            composer require doctrine/lexer "~1.2" --dev --prefer-dist --no-interaction --no-progress
-          elif [ "${{ matrix.doctrine-lexer }}" = "2.1" ]; then
+          if [ "${{ matrix.doctrine-lexer }}" = "2.1" ]; then
             composer require doctrine/lexer "~2.1" --dev --prefer-dist --no-interaction --no-progress
           elif [ "${{ matrix.doctrine-lexer }}" = "3.0" ]; then
             composer require doctrine/lexer "~3.0" --dev --prefer-dist --no-interaction --no-progress
           else
             composer update --prefer-dist --no-interaction --no-progress
           fi
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 75a1d63 and 2e26fab.

📒 Files selected for processing (5)
  • .github/workflows/integration-tests.yml (1 hunks)
  • .github/workflows/unit-tests.yml (1 hunks)
  • ci/php-cs-fixer/config.php (1 hunks)
  • composer.json (1 hunks)
  • tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php (1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 467
File: ci/php-cs-fixer/config.php:21-21
Timestamp: 2025-10-23T14:24:46.219Z
Learning: In PHP CS Fixer, the modern ruleset naming convention uses the pattern `PHP{M}x{m}Migration` (e.g., `PHP7x1Migration`, `PHP8x1Migration`). The older concatenated format without 'x' (e.g., `PHP71Migration`, `PHP81Migration`) is deprecated.
📚 Learning: 2025-10-23T14:24:46.219Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 467
File: ci/php-cs-fixer/config.php:21-21
Timestamp: 2025-10-23T14:24:46.219Z
Learning: In PHP CS Fixer, the modern ruleset naming convention uses the pattern `PHP{M}x{m}Migration` (e.g., `PHP7x1Migration`, `PHP8x1Migration`). The older concatenated format without 'x' (e.g., `PHP71Migration`, `PHP81Migration`) is deprecated.

Applied to files:

  • ci/php-cs-fixer/config.php
📚 Learning: 2025-08-09T15:14:11.841Z
Learnt from: landure
Repo: martin-georgiev/postgresql-for-doctrine PR: 411
File: tests/Integration/MartinGeorgiev/Doctrine/DBAL/Types/DBALTypesTest.php:21-27
Timestamp: 2025-08-09T15:14:11.841Z
Learning: PHP silently ignores unknown attributes in versions that don't recognize them. The #[\Override] attribute introduced in PHP 8.3 can be safely used in code targeting PHP 8.1+ as it will be ignored without errors in 8.1/8.2 and provide override validation in 8.3+. This allows forward compatibility without breaking older PHP versions.

Applied to files:

  • ci/php-cs-fixer/config.php
📚 Learning: 2025-09-01T18:48:28.508Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 434
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/PostGIS/ST_CrossesTest.php:12-31
Timestamp: 2025-09-01T18:48:28.508Z
Learning: When analyzing unit test files in this codebase, always verify the actual file structure and existing patterns before suggesting changes. The ContainsGeometries fixture exists at ./fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php and the PostGIS unit tests use the namespace Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS consistently.

Applied to files:

  • .github/workflows/unit-tests.yml
  • .github/workflows/integration-tests.yml
  • tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php
📚 Learning: 2025-08-12T03:16:13.847Z
Learnt from: landure
Repo: martin-georgiev/postgresql-for-doctrine PR: 412
File: docs/CONTRIBUTING.md:48-51
Timestamp: 2025-08-12T03:16:13.847Z
Learning: In composer.json files, the "require" section defines minimum required versions (mandatory), while the "suggest" section provides recommendations (optional). When documenting supported versions, use the "require" section as the authoritative source for minimum supported versions.

Applied to files:

  • composer.json
📚 Learning: 2025-09-06T22:15:32.757Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 443
File: tests/Unit/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php:169-172
Timestamp: 2025-09-06T22:15:32.757Z
Learning: In martin-georgiev/postgresql-for-doctrine, TextArray users should expect the database to normalize data to strings only. The design assumes that intended PHP data for TextArray is always strings, not other data types, which is a reasonable tradeoff that aligns with PostgreSQL text[] type semantics.

Applied to files:

  • tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php
📚 Learning: 2025-04-11T11:23:44.192Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 340
File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145
Timestamp: 2025-04-11T11:23:44.192Z
Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.

Applied to files:

  • tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.

Applied to files:

  • tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.

Applied to files:

  • tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php
📚 Learning: 2025-08-19T13:07:15.184Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.184Z
Learning: In martin-georgiev/postgresql-for-doctrine, the AVAILABLE-TYPES.md documentation table's second column shows DBAL type names (what TYPE_NAME constants contain and getName() method returns) not PostgreSQL internal catalogue names. Array types use the format like 'text[]', 'jsonb[]', 'inet[]' - not the underscore-prefixed PostgreSQL internal names like '_text', '_jsonb', '_inet'.

Applied to files:

  • tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php
📚 Learning: 2025-08-19T13:07:15.184Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.184Z
Learning: In martin-georgiev/postgresql-for-doctrine, the AVAILABLE-TYPES.md documentation table's second column shows DBAL type names (what getName() method returns) not PostgreSQL internal catalogue names. Array types use the format like 'text[]', 'jsonb[]', 'inet[]' - not the underscore-prefixed PostgreSQL internal names like '_text', '_jsonb', '_inet'.

Applied to files:

  • tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php
📚 Learning: 2025-03-25T01:23:29.749Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 304
File: src/MartinGeorgiev/Utils/DataStructure.php:152-168
Timestamp: 2025-03-25T01:23:29.749Z
Learning: In PostgreSQL text arrays, backslashes must be escaped with triple backslashes. A single backslash in PHP like 'path\to\file' becomes "path\\\to\\\file" in PostgreSQL format.

Applied to files:

  • tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: sloth
  • GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.4
  • GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.4
  • GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.4
  • GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.5
  • GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.4
  • GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.4
  • GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.4
  • GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.2
  • GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.2
  • GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.3
  • GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.3
  • GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.2
  • GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.2
  • GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.3
  • GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.2
  • GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.3
  • GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.2
  • GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.3
  • GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (5)
.github/workflows/unit-tests.yml (1)

46-46: LGTM! PHP matrix correctly updated.

The removal of PHP 8.1 from the test matrix aligns with the PR objective to drop PHP 8.1 support and is consistent with the composer.json PHP requirement change to ^8.2.

composer.json (1)

40-40: LGTM! PHP version constraint correctly updated.

The minimum PHP requirement has been properly updated from ^8.1 <8.6 to ^8.2 <8.6, which aligns with the PR objective to drop PHP 8.1 support.

ci/php-cs-fixer/config.php (1)

22-23: LGTM! PHP 8 migration rules correctly added.

The addition of @PHP8x1Migration and @PHP8x2Migration rulesets is appropriate now that PHP 8.2 is the minimum supported version. The naming convention follows the modern pattern as per coding guidelines.

.github/workflows/integration-tests.yml (1)

46-46: LGTM! PHP matrix correctly updated.

The removal of PHP 8.1 is consistent with the unit-tests workflow and aligns with dropping PHP 8.1 support.

tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php (1)

92-99: Verify that nowdoc indentation changes don't break test assertions.

The indentation of the nowdoc END markers (lines 94 and 99) changes the actual string values being tested. In PHP 7.3+, indenting the closing marker strips that amount of whitespace from all lines in the heredoc/nowdoc. This means leading spaces in the test data on lines 93 and 98 will be removed.

Ensure that the tests still pass with these modified string values and that the change was intentional to adopt PHP 7.3+ flexible nowdoc syntax.

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