Conversation
d63f51f to
9832257
Compare
|
Well... I had some time to experiment today so I took and idea and ran with it. Even if we don't end up using this, it was worth the time to better understand the component. So I ended up separating the PriorityList $routes in the Stacks, renamed them to RouteContainer for clarity, then created a base Interface, and extended Interfaces for SimpleRouteStack and for HttpRouteStack so that subsequently each Route type ends up with a properly-suited RouteInterface (both the RouteMatch and RouteInterfaces both had very http-specific differences). Now this is fully refactored to use PSR-7 requests, and I could remove dependencies - except Laminas\Uri which we need to help assemble routes. I understand this is now a bigger concern in terms of a PR, but as a I said I'm happy to pull back a bit and re-release in sections if that helps us understand how best to approach this. |
|
@simon-mundy Sorry I was just about to review, but I merged #95 so a rebase is needed here. I'm also likely going to merge #92 which will get merged up into 4.0.x once released, so maybe hold off until that's in so rebase is only needed once. |
|
#92 is now released and merged-up |
24fa533 to
b4cd565
Compare
Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
6c6709c to
685b1c2
Compare
|
Thanks @gsteel I've rolled in that change to my PR and it's clearer to see where it sits. |
Signed-off-by: Simon Mundy <46739456+simon-mundy@users.noreply.github.com>
|
Hi @gsteel @froschdesign did you want to discuss any of the code improvements? |
- Updated tests to reflect new factory change - Removed unnecessary ServiceListener check - Changed string literal to class check in RouterConfigTrait.php Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Signed-off-by: Simon Mundy <46739456+simon-mundy@users.noreply.github.com>
- Minor fixes required after merge in github - Cleanup psalm baseline Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
|
@simon-mundy imo you could split your pr in multiple one, the review would be easier and your work could be merged faster
|
|
@fezfez I haven't split these before - do I create individual PRs for this? For each item you've listed? It may or may not be easy to separate everything but I'll see what I can do. |
Description
This PR addresses issues #75 and #76 and seeks to modernise the codebase for a 4.0.x release.
The existing Filter and View components have been used as guidance, and I have been
aggressive in seeking better psalm compliance, refactoring of code and tests and
removing outdated/deprecated code.
Breaking Changes
RoutePluginManager
Route Classes
finalmodifier where applicableRoutePriorityTraitfor consistent priority handlingRouteConfigTraitfor standardized option processing and iterator conversionsetPriority()/getPriority()added to RouteInterfaceCode Quality
final readonlyfinalArrayUtils::iteratorToArray()with trait-based implementationNOTES