-
Notifications
You must be signed in to change notification settings - Fork 61
add custom rector rule to cleanup RouteBuilder #341
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
base: 5.x
Are you sure you want to change the base?
Conversation
826e9cd to
6a4bb79
Compare
|
To not make the configuration too complicated i just hard coded the scope specific logic in that rule |
tests/test_apps/upgraded/RectorCommand-testApply53/src/SomeTest.php
Outdated
Show resolved
Hide resolved
tests/test_apps/upgraded/RectorCommand-testApply53/src/SomeTest.php
Outdated
Show resolved
Hide resolved
| $routes->scope('/api', function (RouteBuilder $routes): void {}); | ||
| CODE_SAMPLE, | ||
| <<<'CODE_SAMPLE' | ||
| $routes->scope(path: '/api', options: [], callback: function (RouteBuilder $routes): void {}); |
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 don't like the reaming of $params to $options, we use the term "route params".
We need to first update the method and make $params argument default to empty array and make $callable not nullable.
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'll leave those changes to the the core to you @ADmad
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 seems only resources() should be $options.
6a4bb79 to
233bb80
Compare
|
Do you think #367 is complete? |
8d60f3c to
c3280a4
Compare
Refs: cakephp/cakephp#18002
There is still one problem though which I have not yet tackled:
does not work, as
paramsneeds to be set as an empty array (which we don't have as a default value only on that method, not the others)