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

Trigger deprecation in AnnotationDriver only if PHP >= 8.0 #10760

Closed
wants to merge 1 commit into from

Conversation

fancyweb
Copy link

@fancyweb fancyweb commented Jun 5, 2023

Hello, I think it would make sense to not trigger this deprecation when it's impossible to migrate to the new AttributeDriver because of the PHP version. The deprecation is not actionable: I don't really consider switching to the XML driver a viable migration.
People running PHP < 8.0 won't be able to use Doctrine 3.0 anyway since it requires PHP 8.1.

greg0ire
greg0ire previously approved these changes Jun 5, 2023
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Sounds sensible.

@derrabus
Copy link
Member

derrabus commented Jun 5, 2023

tbh, I'd leave everything as it is. The deprecation can be silenced individually, if it annoys you.

The deprecation is not actionable: I don't really consider switching to the XML driver a viable migration.

Well, first of all: yes, XML is a viable option. Apart from that, the preferred migration path would be to upgrade to PHP 8 and switch to attributes. And I consider this path to be actionable and if it isn't, the impediment is not on the ORM's side.

People running PHP < 8.0 won't be able to use Doctrine 3.0 anyway since it requires PHP 8.1.

They don't have to fix any deprecations then for as long as it's "impossible" for them to migrate to ORM 3. I realize that not every deprecation is trivial to fix and deprecations don't need to be fixed right away. That is why the deprecation framework that we use allows apps to silence deprecations by identifier.

Really nobody should be on PHP 7 anymore. And that deprecation could even add more arguments for moving forward to PHP 8. If we'd hide it on PHP 7, people might be surprised to see new deprecations after having upgraded to PHP 8.

@greg0ire greg0ire dismissed their stale review June 5, 2023 12:05

I didn't put into words a lot of what derrabus wrote here, as I thought it would be maybe a bit too strict, but since derrabus also thinks we shouldn't merge this, I'll dismiss my approval. I think we should close this.

@fancyweb
Copy link
Author

fancyweb commented Jun 5, 2023

Thanks for your fast reviews, I'll close.

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