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

Add parameter type declarations #467

Merged
merged 1 commit into from Dec 18, 2022

Conversation

greg0ire
Copy link
Member

No description provided.

@greg0ire greg0ire force-pushed the param-type-declarations branch 2 times, most recently from 7d60371 to c20b3c3 Compare December 17, 2022 16:27
@derrabus
Copy link
Member

We should bump to PHP 7.2 at least, otherwise it's tough for downstream projects to support v1 and v2.

@greg0ire
Copy link
Member Author

greg0ire commented Dec 17, 2022

🤔 if I bump to 8.1 I can add more parameter type declarations (some with mixed, some with union types). Then, the ORM can still add compatibility with 2.0, and allow lexer 3.

@greg0ire greg0ire force-pushed the param-type-declarations branch 3 times, most recently from 3969f58 to 9fd489f Compare December 17, 2022 19:29
@derrabus
Copy link
Member

🤔 if I bump to 8.1 I can add more parameter type declarations

Yes, then again a PHP-8.1-only version of doctrine/annotations feels a bit pointless given that attributes exist.

@greg0ire
Copy link
Member Author

If you want to migrate from annotations to attributes, you have to upgrade to PHP 8 first. If you do, you are probably not going to update to PHP 8.0, right?

@greg0ire
Copy link
Member Author

Anyway, I agree it doesn't make a lot of sense, the thing is, I have to release 2.0 with doctrine/lexer 3 support soon, because that way, I can add support for doctrine/lexer 3 in ORM 2.15. I thought I might as well do this, but if you feel I should just tag 2.0 now, let me know.

@derrabus
Copy link
Member

I think, the PR does make sense. It's just that I'd bump to PHP 7.2 (to gain parameter type widening) or maybe 7.4 if you like. But not beyond that.

@greg0ire
Copy link
Member Author

Ok, let's bump to 7.2 then 👍

derrabus
derrabus previously approved these changes Dec 18, 2022
This allows us to simplify some dependencies constraints, and to add
parameter type declarations.
@greg0ire greg0ire merged commit 96c1475 into doctrine:2.0.x Dec 18, 2022
@greg0ire greg0ire deleted the param-type-declarations branch December 18, 2022 19:38
@greg0ire greg0ire added this to the 2.0.0 milestone Dec 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants