Skip to content
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

Implement https://php.watch/versions/8.3/typed-constants #220

Closed
wants to merge 6 commits into from

Conversation

rossaddison
Copy link

Q A
Is bugfix?
New feature? ✔️
Breaks BC?

Perhaps
https://stackoverflow.com/questions/28865066/phpunit-fails-asserting-that-two-arrays-are-equal-but-shows-no-difference

has relevance here.

"Found the issue. Elements were not in the same order. A bit annoying though that the output doesn't show it."

"I have replaced assertEquals with:

$this->assertTrue($this->similar_arrays($arr1, $arr2));

protected function similar_arrays($a, $b) {
    if(is_array($a) && is_array($b)) {
        if(count(array_diff(array_keys($a), array_keys($b))) > 0)
            return false;

        foreach($a as $k => $v) {
            if(!$this->similar_arrays($v, $b[$k]))
                return false;
        }

        return true;
    }
    else
        return $a === $b;
}"

The test passes when 'title' => 'string' is inserted so that the arrays are equal in terms of 'key-value pair' count and not in the same order.
@samdark samdark requested a review from roxblnfk January 25, 2025 09:02
Copy link
Member

@roxblnfk roxblnfk left a comment

Choose a reason for hiding this comment

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

Hello. Even though we follow strict typing, we also try to keep a balance in the importance of changes.
Setting types for constants is not worth upgrading PHP to version 8.3.
Moreover, Cycle ORM is compatible with PHP 8.1.

@vjik
Copy link
Member

vjik commented Jan 25, 2025

You can use @psalm-suppress MissingClassConstType annotation to suppress psalm error. For example:

/**
 * @psalm-suppress MissingClassConstType
 */
final public const TYPE_SUMMARY = 'summary';

@@ -39,20 +39,11 @@ jobs:
- windows-latest

php:
- 8.1
- 8.2
Copy link
Member

Choose a reason for hiding this comment

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

We still support 8.2+

@rossaddison rossaddison closed this Feb 1, 2025
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.

4 participants