-
-
Notifications
You must be signed in to change notification settings - Fork 55
feat!: drop support for PHP 8.1 #468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # .github/workflows/ci.yml
# Conflicts: # composer.json
WalkthroughThe 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
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this 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
📒 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.ymltests/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.6to^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
@PHP8x1Migrationand@PHP8x2Migrationrulesets 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.
Summary by CodeRabbit