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

Migrate files with the least changes to PHP 8 #10367

Merged
merged 1 commit into from Jan 3, 2023

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Jan 3, 2023

These are the lowest hanging fruits I could find after running Rector: I looked for files with a diff of 2 lines.
I did not include some changes that I find controversial, such as marking some constants as final when we should maybe consider making classes themselves final.

@@ -102,9 +102,9 @@ public function count(): int
/**
* {@inheritdoc}
*
* @psalm-return Traversable<array-key, T>
* @psalm-return Traversable<array-key, T>&ArrayIterator
Copy link
Member Author

Choose a reason for hiding this comment

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

Here, we can't just use ArrayIterator<array-key, T> because that would make T appear in an invariant position, while it's declared as covariant.

Copy link
Member

Choose a reason for hiding this comment

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

Do we want ArrayIterator to be part of the contract? I don't see a reason to change the return type.

Copy link
Member Author

Choose a reason for hiding this comment

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

🤔 hum you're right, it makes no sense to leak that info.

@greg0ire greg0ire marked this pull request as ready for review January 3, 2023 22:18
@greg0ire greg0ire added this to the 3.0.0 milestone Jan 3, 2023
@greg0ire greg0ire added this to In progress in PHP 8 Migration via automation Jan 3, 2023
These are the lowest hanging fruits I could find after running Rector: I
looked for files with a diff of 2 lines.
I did not include some changes that I find controversial, such as
marking some constants as final when we should maybe consider making
classes themselves final.
@greg0ire greg0ire merged commit 3a10714 into doctrine:3.0.x Jan 3, 2023
PHP 8 Migration automation moved this from In progress to Done Jan 3, 2023
@greg0ire greg0ire deleted the php8-migration branch January 3, 2023 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants