-
-
Notifications
You must be signed in to change notification settings - Fork 409
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
[tests] bring test suite up to PHP8 standards #1129
Conversation
4f6a482
to
f7560c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comments, but looks good 👍
@@ -70,6 +66,8 @@ public function addExtraDependencies(string ...$packages): self | |||
|
|||
public function setRequiredPhpVersion(int $version): self | |||
{ | |||
@trigger_deprecation('symfony/maker-bundle', 'v1.44.0', 'setRequiredPhpVersion() is no longer used and will be removed in a future version.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we won't need this again? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. But we are not using it within the code base at the moment. Because the class isn't internal - I figured we'd throw the deprecation. And if/when we need to use this (PHP 9
ish) - we can either remove the deprecation or add a replacement internal
method that gives us a bit more freedom.
I think we are going with a similar strategy w/ the PHPCompatUtil
- we don't have a need for now (probably wont until PHP 9ish). So when we introduced the util in non-internal classes, we added a deprecation that "hey, you need to supply this util.. it'll be mandatory in the next version..." Now that we don't need it in say MakeWhatever::class, I changed the wording on the deprecation "hey, you dont need to supply this..."
That strategy kinda sucks, but I can't think of a better way w/ non internal classes. Any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works for me 👍
9a62803
to
669c325
Compare
Thanks Jesse! |
No description provided.