-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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(App): Remove registerRoutes method #42678
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
OpenAPI is failing because it previously failed to discover all the routes in user_ldap so they are missing now and need to be documented. |
/** @var string[] */ | ||
private $controllerNameCache = []; | ||
|
||
protected $rootUrlApps = [ |
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.
???
How is this implemented now?
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.
Check RouteParser, it has exactly the same apps listed and the same logic 🤷♀️. Think someone just copied parts of the implementation which lead to this logic duplication.
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.
🙈
Time to migrate the tests over then? |
Yes, but I took a look and I'm not really sure what the tests are trying to achieve. Are they testing that the routes are correctly parsed or are they checking that the routes are added correctly to the Router? |
Summary
Removes the deprecated App::registerRoutes method.
The RouteConfig class is no longer used so I deleted it as well including it's tests.
For some reason much of the implementation was the same as RouteParser, but that one has no tests 🤔.
Checklist