Draft
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request represents a major architectural refactoring that replaces the Kohana framework application with a new custom Sprout application system. The changes introduce a two-tier application architecture with BaseApp (core routing and buffering) and Sprout\App (Sprout CMS-specific behaviors). A lightweight WelcomeApp is provided for the initial setup system.
Changes:
- Introduces new
BaseAppandAppclasses that handle request routing, controller execution, and output buffering through lifecycle events - Replaces Kohana's routing system with a simplified router based on karmabunny/router with Action objects instead of string-based routes
- Refactors
Configclass to extend karmabunny/kb'sBaseConfigwith configurable search paths for config files
Reviewed changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/core/BaseApp.php | New base application class providing core routing, buffering, and lifecycle management |
| src/core/ControllerInterface.php | New controller interface defining the _run method contract |
| src/sprout/App.php | Main Sprout application extending BaseApp with CMS-specific initialization |
| src/sprout/Welcome/WelcomeApp.php | Minimal application for the Welcome setup system |
| src/sprout/core/Kohana.php | Gutted to extend App, most methods removed or deprecated |
| src/sprout/core/Bootstrap.php | Deleted - bootstrap logic moved to new app.php |
| src/bootstrap/app.php | New bootstrap file that instantiates and runs the configured app |
| src/bootstrap/welcome.php | New bootstrap for Welcome module that uses WelcomeApp |
| src/bootstrap/kohana.php | Updated to initialize App system and maintain backward compatibility |
| src/bootstrap.php | Updated to use new bootstrap flow |
| src/sprout/Helpers/Config.php | Refactored to extend BaseConfig from karmabunny/kb |
| src/sprout/Helpers/Router.php | Gutted - most methods removed, static properties deprecated |
| src/sprout/Helpers/Request.php | Added findUri() and enhanced getQueryParams() for CLI support |
| src/sprout/Helpers/PageRouting.php | Updated to use PostRoutingEvent and create Action objects |
| src/sprout/Helpers/Url.php | Updated redirect to use App::instance()->shutdown() |
| src/sprout/Helpers/SubsiteSelector.php | Now sets Config::$paths for skin-specific configs |
| src/sprout/Helpers/Modules.php | Now sets Config::$paths for module configs |
| src/sprout/Helpers/Errors.php | Updated to use App::closeBuffers() and App::instance()->shutdown() |
| src/sprout/Helpers/CliServer.php | Updated to use Request::findUri() |
| src/sprout/Helpers/Session.php | Updated event listener from Kohana::class to App::class |
| src/sprout/Helpers/SessionStats.php | Updated event listener from Kohana::class to App::class |
| src/sprout/Helpers/Sprout.php | Delegated send() to App::send() |
| src/sprout/Controllers/BaseController.php | Updated to implement ControllerInterface and trigger events on self::class |
| src/sprout/Controllers/DbToolsController.php | Updated event listener from Kohana::class to App::class |
| src/sprout/Welcome/Controllers/WelcomeController.php | Added Route attributes for new routing system |
| src/sprout/Welcome/config/routes.php | Simplified to use Action-based routes |
| src/sprout/Welcome/config/config.php | New config file overriding app class to WelcomeApp |
| src/sprout/Events/BootstrapEvent.php | New event for application bootstrap phase |
| src/sprout/Events/PreRoutingEvent.php | Added method property |
| src/sprout/Events/PostRoutingEvent.php | Added method and action properties |
| src/sprout/Events/PreControllerEvent.php | Added controller, method, and arguments properties |
| src/sprout/Events/PostControllerEvent.php | Added response property |
| src/sprout/Events/SendHeadersEvent.php | Deprecated in favor of headers_sent() |
| src/sprout/core/Event.php | Updated to register events on App::class instead of Kohana::class |
| src/sprout/config/config.php | Added app class config, removed obsolete Kohana settings |
| src/sprout/sprout_load.php | Removed SessionStats::init() and CoreAdminAuth import |
| composer.json | Added src/core autoload path and updated karmabunny/kb requirement |
| composer.lock | Updated karmabunny/kb to v4.61.44 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1979b83 to
4369f35
Compare
This often captures fatal errors that are not logged elsewhere.
To replace kohana routing + buffers.
Required for visor tests.
This has several advantages: - natural cascading - test coverage - clarity for include paths
This mirrors the findUri() behaviour from the old Router.
4369f35 to
f2a4e87
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.
I've been somewhat obsessively refining this branch for a few days.
This entirely replaces the Kohana application with our own.
A few other pieces:
Sprout App
The new Sprout application is in two parts. The core
BaseAppand the SproutApp.The base component is purely about routing + buffering. There's no assumptions about anything like modules, services, etc.
The Sprout App then applies all it's behaviours into the lifecycle events.
The aim here is two things - first is the isolation of behaviours. This aids maintainability and testing the application in isolation of side-effects from larger Sprout components.
The second lets us modify the bootstrap for projects that don't need/want the entire CMS. This is demonstrated by the WelcomeApp.
Draft because
This builds on #177 and I don't want this accidentally merged.