Skip to content

⬆️ drop php 7 #76

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

⬆️ drop php 7 #76

wants to merge 1 commit into from

Conversation

garak
Copy link
Member

@garak garak commented Jun 30, 2025

Fix #67

@garak garak requested a review from Copilot June 30, 2025 07:17
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

A concise update dropping PHP 7 support and upgrading the codebase and test suite to PHP 8.1+, leveraging new language features and aligning CI and coding standards.

  • Bumped minimum PHP version to 8.1 and updated composer dependencies & CI matrix
  • Refactored core classes to use constructor property promotion and implement \Stringable
  • Migrated PHPUnit data providers to attributes, added trailing commas, and marked most tests final

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.

Show a summary per file
File Description
composer.json Raised PHP requirement to ^8.1; updated dev dependencies
.github/workflows/run-tests.yml Removed PHP 7 from matrix; added extensions & flags
phpunit.xml Updated schema location; replaced listeners with extension
.php-cs-fixer.php Switched to @PHP81Migration; refined trailing-comma rule
src/Piece.php Added \Stringable and used property promotion
src/Move.php Added \Stringable, property promotion, trailing commas
src/History.php Converted constructor to property promotion
src/Entry.php Applied property promotion to all constructor params
src/Output/Link.php Converted to property promotion on constructor
src/Output/ImagineOutput.php Added trailing comma to parameters
src/Chess.php Added trailing comma in recordMove call
tests/PieceTest.php Made test class final; added JSON_THROW_ON_ERROR
tests/PiecePlacementTest.php Made test class final
tests/PerftTest.php Made test class final; switched to #[DataProvider]
tests/MoveTest.php Imported DataProvider; added trailing commas
tests/MiscTest.php Made test class final
tests/GameTest.php Made test class final
tests/FenTest.php Added #[DataProvider] annotation
tests/ConstructorTest.php Made test class final
tests/ChessPublicator.php Made test class final; added trailing comma
tests/AttackTest.php Made test class final
Comments suppressed due to low confidence (3)

src/Output/Link.php:13

  • [nitpick] Using $class as a property name can be confusing (it’s a reserved word). Consider renaming it to something more descriptive like $className or $cssClass.
    public function __construct(public ?string $class, public ?string $url)

tests/MoveTest.php:11

  • The DataProvider import is not used in this file. Either remove it or add appropriate #[DataProvider] annotations to tests that need it for consistency.
use PHPUnit\Framework\Attributes\DataProvider;

tests/MoveTest.php:14

  • [nitpick] Most test classes in this suite are declared final. Consider marking MoveTest as final as well to maintain consistency.
class MoveTest extends TestCase

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.

Drop support for PHP 7.4?
1 participant