Repository navigation
Issue/538 Group the stages by adapter and give the runtime stages lifecycle events - #591
Conversation
The web boot stages that only the web adapter runs move to Adapters/Web/Stages/Boot and the web runtime stages to Adapters/Web/Stages/Runtime, with their tests. Only namespaces and imports change.
The console boot stages move to Adapters/Console/Stages/Boot and RunCommandStage to Adapters/Console/Stages/Runtime, with their tests. src/App/Stages/Boot keeps only the stages both adapters run. Only namespaces and imports change.
…vents The six web runtime stages declare web.<stage>.before / .after and RunCommandStage declares console.command.before / .after; the pipeline dispatches them with the runtime context.
The boot belongs to the framework: no boot stage declares BEFORE/AFTER, and a test guards it for every boot folder. App helper listeners hook into the runtime events instead. The pipeline no longer checks whether the event() helper is loaded, since runtime stages run after the helpers.
…out in the changelog
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 11.54% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 34 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Linked Issues check |  |
The supplied PR evidence supports the coding changes for #538. The current paths group the adapter stages and tests. The old Web and Console stage paths are absent. Runtime stages declare the specifie… |
Provide reviewable results for composer test, composer cs:check, composer phpstan, and composer rector:check, or equivalent CI evidence. Provide a readable merge-base diff if verification of the complete change set is required. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly summarizes the primary changes: grouping stages by adapter and adding lifecycle events to runtime stages. Out of Scope Changes check The supplied change summary ties the source, test, pipeline, and changelog changes to #538. The moved stages and tests support the grouping objective. The runtime event tests support the lifecycle obj…Full details: Out of Scope Changes check
-
Explanation
The supplied change summary ties the source, test, pipeline, and changelog changes to
#538. The moved stages and tests support the grouping objective. The runtime event tests support the lifecycle objective. No change demonstrates Debugbar integration, deferred response tasks, or a native non-exiting Quantum console.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 11.54% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 34 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Full details: Linked Issues check
-
Explanation
The supplied PR evidence supports the coding changes for
#538. The current paths group the adapter stages and tests. The old Web and Console stage paths are absent. Runtime stages declare the specified events. The pipeline sends the context payload. Boot stages no longer declare events. The changelog documents the event names and lifecycle rules. The tests cover ordering, stop and failure behavior, payloads, request clearing, and console behavior. The evidence does not show results forcomposer cs:check,composer phpstan, orcomposer rector:check. The merge-base diff is unavailable, so these requirements cannot be verified.Resolution
Provide reviewable results for
composer test,composer cs:check,composer phpstan, andcomposer rector:check, or equivalent CI evidence. Provide a readable merge-base diff if verification of the complete change set is required.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Closes #538
What changed
Stage layout: the stages are grouped by the adapter that runs them. Only namespaces and imports change.
src/App/Stages/Webandsrc/App/Stages/Consoleare gone, and the tests mirror the new paths.Runtime events: the six web runtime stages declare
web.preflight,web.route,web.not_found,web.prepare,web.dispatchandweb.send, andRunCommandStagedeclaresconsole.command, each with.beforeand.after. The pipeline dispatches them with['context' => $context].No boot events:
LoadEnvironmentStage,LoadAppConfigStage,SetupErrorHandlerStage,InitHttpStageandLoadModulesStageno longer declareboot.*events, and a test fails if any boot stage declaresBEFOREorAFTER.Pipeline:
dispatchStageEvent()no longer checks whether theevent()helper is loaded; only runtime stages declare events, and they run after the helpers.Changelog: the 3.1.0 "Added" line describes the runtime events instead of the boot events, and the lifecycle entry describes the stage layout.
Why
App code hooks into the running app, not into the framework's own setup: the boot events let a listener from a helper file run in the middle of the boot, before the config or the modules exist, and on a broken
.envthe console boot stops before most of them. The runtime stages declared no events at all, so there was nothing to listen to while a request or a command runs.With #589 a stage event means one thing: a stage that runs fires its events, a stage that does not run fires none. So a listener never has to check
isStopped():web.*pair, in stage orderweb.preflight.*,web.send.*web.preflight.*,web.route.*,web.not_found.*,web.send.*.before; no.after, noweb.send.*web.send.afterfires after the request state is cleared.console.command.afterfires only with an application that does not exit after the command; Symfony's defaultApplication::run()exits.Grouping the stages first means every event constant lands on its stage in its final place.
Breaking changes
None compared with 3.0.3: the boot events and the 3.1.0 stage namespaces were never released. Code that goes through
AppFactoryis unaffected.Testing
PipelineTest: the event names of the seven runtime stages, and a guard that scans the three boot stage folders forBEFORE/AFTER(it failed while the boot stages still declared them).WebAppAdapterTest: the exact event order for a matched route, an OPTIONS request, a request without a route and a stage that throws; the listeners get theWebContext; the request is cleared inweb.send.after.ConsoleAppAdapterTest:console.command.beforeand.afteraround a command with theConsoleContext, and only.beforefor an unknown command.web.preflight.beforeinstead ofboot.modules.before; the boot event name test and the test for skipping events while theevent()helper is not loaded are removed.master: the web requests (/,/api/posts, OPTIONS, 404 as HTML and JSON) and the console cases (valid, missing and malformed.env) give identical results. A temporary app helper listener receivesboot.modules.afteronmasterandweb.preflight.before/web.send.afterper request (also OPTIONS) andconsole.command.beforeforphp qt liston this branch.Summary by CodeRabbit
New Features
Breaking Changes