-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
refactor: upgrade to PHP 8.1 with rector #7964
Conversation
65e6f8b
to
770bc20
Compare
@samsonasik When I ran rector first, the following change did not happen. $ vendor/bin/rector
119/119 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
1 file with changes
===================
1) system/Router/AutoRouterImproved.php:410
---------- begin diff ----------
@@ @@
{
try {
$refClass = new ReflectionClass($this->controller);
- } catch (ReflectionException $e) {
+ } catch (ReflectionException) {
throw PageNotFoundException::forControllerNotFound($this->controller, $this->method);
}
----------- end diff -----------
Applied rules:
* RemoveUnusedVariableInCatchRector (https://wiki.php.net/rfc/non-capturing_catches) |
@kenjis rector sometime need twice run as the code got consecutive run on single rule or just reprinted on next rule apply to specific node to avoid infinite loop |
@samsonasik I got it. Thanks! |
8927507
to
4ee7742
Compare
4ee7742
to
9f21a47
Compare
Rebased and re-run rector. |
I'm not going to review all the changes, but what I spot-checked looks good and pipeline is passing. This will inevitably be a big merge conflict before we are ready for the next minor release. I suggest you get the automated changes in place via Rector (done) then consolidate the manual revisions to a commit(s) that can be cherry-picked after rerunning Rector. Excited to be on 8+! Thanks for your work on this - and shoutout to the attention from @TomasVotruba 😍 and the tireless effort from the whole Rector team, including our own @samsonasik. |
@MGatner I don't understand what you exactly mean. |
No, sorry to be unclear! I think what you are doing is great - I was just noting that it might not be worthwhile to keep up with conflicts on this PR, but rather wait and rerun Rector right before we are ready for release. |
@MGatner I got your point. Thanks. |
9f21a47
to
53e936f
Compare
👋 Hi, @kenjis! |
53e936f
to
9063773
Compare
7ceb7e4
to
4366d27
Compare
…ct codes are not changed
…isting product codes are not changed
…extent that existing product codes are not broken
68d8a81
to
848b980
Compare
848b980
to
3c2f995
Compare
3c2f995
to
71124e7
Compare
👋 Hi, @kenjis! |
Due to the difficulty in resolving the conflicts, I close this PR. |
Description
Supersedes #6923
See #6921 and #8042
Checklist: