-
-
Notifications
You must be signed in to change notification settings - Fork 429
[Php85] Rename deprecated PDO constants and methods #7125
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
[Php85] Rename deprecated PDO constants and methods #7125
Conversation
|
Thanks. Could you resolve conflicts? |
|
Will do so tomorrow for all of my PHP 8.5 PRs that need conflict resolution. |
config/set/php85.php
Outdated
| $rectorConfig->ruleWithConfiguration( | ||
| RenameClassConstFetchRector::class, | ||
| [ | ||
| new RenameClassAndConstFetch('PDO', 'DBLIB_ATTR_CONNECTION_TIMEOUT', 'Pdo\Dblib', 'ATTR_CONNECTION_TIMEOUT'), |
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.
downgrade rule is needed for this, to downgrade constant to previous available constant, since the constant exists since php 8.4
You can see example of it
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.
@samsonasik Does this really need a new rector implementation or can't the inverse RenameClassAndConstFetch config be used in config/set/downgrade-php85.php?
PS: I just saw some other rule sets also using "normal" rector rename rules, thus I will do the same.
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.
when downgrading code, that exists in old and removed later, or only exists in future php version, the snippet code generate can be used, eg this example
Then use inline code parser to cover that
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.
@samsonasik Sadly, I can't connect your comment to this PR but I created rectorphp/rector-downgrade-php#304 where we can tackle this discussion and if that solution has issues relating to your example from the previous comment.
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.
Once code downgraded, the code should keep working even on the future php version is met, eg, downgraded from 8.1 to 8.0, when downgraded code opened in php 8.1, it should keep working.
Php is unfortunatelly not semver, so some tweak for this kind of usage seems needed, like if exists check.
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.
I am unsure exactly whether rectorphp/rector-downgrade-php#304 meets these requirements. If it does not, someone else should tackle this topic then, please. At this point in time, I do not want to spend my time on writing code to downgrade PHP code.
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.
It's the same rules, just inverted renames.
fcd5904 to
ff5fe75
Compare
|
Let's merge this as it is, as downgrade of pdo is quite a niche feature. We can keep those separately. Thanks for the feature 👌 |
|
This pull request has been automatically locked because it has been closed for 150 days. Please open a new PR if you want to continue the work. |
No description provided.