Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Removes the parameter synchronization mechanism and shifts argument injection/resolution to respect/parameter, while updating routines/routes/tests/docs to reflect positional route parameters and PSR-7/PSR-11-based injection.
Changes:
- Deleted Param Sync abstractions (
ParamSynced,AbstractSyncedRoutine,ResolvesCallbackArguments) and updated routines to resolve arguments viaRespect\Parameter\Resolver. - Updated dispatch/routine execution flow to call routine methods directly and use
DispatchContext::resolver()for argument resolution. - Updated tests and documentation to reflect positional route params (no name-based sync) and callable-only routine callbacks.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Stubs/AbstractRoutine.php | Aligns stub callback typing with callable-only routines. |
| tests/Routines/ByTest.php | Updates coverage references and switches to invokable instance usage. |
| tests/Routines/AbstractSyncedRoutineTest.php | Removes tests tied to deleted Param Sync behavior. |
| tests/Routines/AbstractRoutineTest.php | Updates constructor argument expectations to callable / TypeError behavior. |
| tests/RouterTest.php | Adjusts proxy parameter expectations to positional (non-synced) behavior. |
| tests/Psr7InjectionTest.php | Reorders/keeps PSR-7 injection coverage for through routines. |
| tests/DispatchContextTest.php | Removes Param Sync-specific test coverage and helpers. |
| src/Routines/When.php | Migrates When to AbstractRoutine + Resolver-based argument resolution. |
| src/Routines/Through.php | Migrates Through to AbstractRoutine + Resolver-based argument resolution. |
| src/Routines/ParamSynced.php | Deletes Param Sync interface. |
| src/Routines/By.php | Migrates By to AbstractRoutine + Resolver-based argument resolution. |
| src/Routines/AuthBasic.php | Switches callback reflection/type checks and arg resolution to Resolver. |
| src/Routines/AbstractSyncedRoutine.php | Deletes Param Sync base class. |
| src/Routines/AbstractRoutine.php | Enforces callable-only callbacks (constructor + getter typing). |
| src/RoutinePipeline.php | Calls when/by/through directly (removes DispatchContext::routineCall). |
| src/Routes/ControllerRoute.php | Uses DispatchContext::resolver() for controller method invocation args. |
| src/Routes/Callback.php | Uses Resolver::reflectCallable() and DispatchContext::resolver() for args. |
| src/Routes/AbstractRoute.php | Removes ResolvesCallbackArguments usage and getTargetReflection(). |
| src/ResolvesCallbackArguments.php | Deletes legacy PSR-7 argument injection trait. |
| src/DispatchContext.php | Implements ContainerInterface and exposes resolver() for injection. |
| docs/README.md | Updates docs to remove param sync claims and document positional params. |
| composer.json | Adds psr/container and respect/parameter dependencies. |
Comments suppressed due to low confidence (1)
src/Routines/AuthBasic.php:26
AuthBasic::__construct()acceptsmixed $callback, but the parentAbstractRoutineconstructor now requirescallable. Updating this parameter type tocallablewould make the public API clearer and surface invalid callbacks earlier (without relying on the parent call’s TypeError).
public function __construct(public string $realm, mixed $callback)
{
parent::__construct($callback);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #178 +/- ##
============================================
- Coverage 96.91% 96.66% -0.25%
+ Complexity 437 403 -34
============================================
Files 33 31 -2
Lines 1101 1020 -81
============================================
- Hits 1067 986 -81
Misses 34 34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Parameter synchronization has been removed, favoring the usage of PSR7 interfaces for exchange of data between routines and routes if necessary, allowing the same controller splitting technique. Respect\Rest now uses Respect\Parameter for its Reflection duties, and the main DispatchContext exposes a valid PSR11 container with its injectable dependencies.
48d1f22 to
fd6436c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parameter synchronization has been removed, favoring the usage of PSR7 interfaces for exchange of data between routines and routes if necessary, allowing the same controller splitting technique.
Respect\Rest now uses Respect\Parameter for its Reflection duties, and the main DispatchContext exposes a valid PSR11 container with its injectable dependencies.