Skip to content

Conversation

@nunomaduro
Copy link
Member

Currently the interface/contract Application is specified on the following code:

abstract class ServiceProvider
{
    /**
     * The application instance.
     *
     * @var \Illuminate\Contracts\Foundation\Application
     */
    protected $app;

Problem:: There are service providers that assume that the $app implementation have the method runningInConsole. For example the MailServiceProvider on the method:

 /**
     * Register the Markdown renderer instance.
     *
     * @return void
     */
    protected function registerMarkdownRenderer()
    {
        if ($this->app->runningInConsole()) {

Since runningInConsole do not exist on the contract, I propose is just specify on the php docs that service providers are instantiated with instance of Illuminate\Foundation\Application.

@taylorotwell taylorotwell merged commit 55e6023 into laravel:5.4 Mar 8, 2017
@ghost
Copy link

ghost commented Mar 15, 2017

Illuminate/support is sometimes used with its own implementation of \Illuminate\Contracts\Foundation\Application, without using laravel/framework.
Therefore, I believe that this change is wrong.

taylorotwell pushed a commit that referenced this pull request Apr 4, 2017
…n Contract (#18658)

* Adds the `runningInConsole` method to the Foundation Application Contract

* Revert "Fixes phpdocs (#18248)"

This reverts commit 55e6023.
ibpavlov added a commit to ibpavlov/framework that referenced this pull request Jun 1, 2017
Restore phpDocs original parameter class. 

This is a rollback of another Pull-Request (laravel#18248) that breaks other Projects not using the Illuminate\Foundation.
taylorotwell pushed a commit that referenced this pull request Jun 1, 2017
Restore phpDocs original parameter class. 

This is a rollback of another Pull-Request (#18248) that breaks other Projects not using the Illuminate\Foundation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants