-
Notifications
You must be signed in to change notification settings - Fork 5
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
[1.1] Overhauling the bootloader to be more flexible #541
Conversation
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.
Some things for your review.
@@ -28,8 +28,12 @@ class Model_Service_Provider extends Service_Provider { | |||
* Register the service provider. | |||
*/ | |||
public function register(): void { | |||
Model::set_event_dispatcher( $this->app['events'] ); | |||
} |
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.
Do you mean to leave this method empty 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.
Yep!
$dirname = basename( dirname( $file->getRealPath() ) ); | ||
$environment = match ( $dirname ) { | ||
// Ignore the 'config' directory as an environment. | ||
'config', 'configuration' => null, |
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.
Not really sure, but do any of the changes for loading and merging configuration in this file have performance considerations we should think about?
* @param class-string<Contracts\Http\Kernel>|null $http HTTP kernel class. | ||
*/ | ||
public function with_kernels( string $console = null, string $http = null ): static { | ||
if ( $console && ! in_array( Contracts\Console\Kernel::class, class_implements( $console ), true ) ) { |
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.
Would a ! class_exists( $handler )
also make sense here? Or would this already be caught? Same question for with_exception_handler
method below.
Co-authored-by: Damian <4309872+attackant@users.noreply.github.com>
Allow the bootloader to configure an application without any additional arguments. Also, allow the application to be configured through the bootloader. This will allow any application to truly use the framework's features with just:
Previously, this was a goal and worked but not exactly. The queue didn't work well without a kernel and some more configuration. All of this is now behind us and we can call the bootloader in another application and use the queue/database/etc.
With the changes in this PR we can simplify
bootstrap/app.php
to this: