Skip to content
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

[5.6] Adds nunomaduro/collision on composer require-dev #4514

Merged
merged 1 commit into from
Dec 19, 2017
Merged

[5.6] Adds nunomaduro/collision on composer require-dev #4514

merged 1 commit into from
Dec 19, 2017

Conversation

nunomaduro
Copy link
Member

@nunomaduro nunomaduro commented Dec 17, 2017

This PR proposes the addition of the package Collision as dev dependency of every Laravel app. Collision is an detailed & intuitive error handler framework for PHP console applications, and includes an adapter/service-provider for Laravel.

The package project is https://github.com/nunomaduro/collision and bellow is an screenshot example.

example-2
Those are the Collision dependencies:

    "require": {
        "php": "^7.1",
        "filp/whoops": "^2.1.4",
        "symfony/console": "~2.8|~3.3|~4.0"
    }

Concerning the integration Laravel, If the exception implements the Symfony\Component\Console\Exception\ExceptionInterface, Collision still uses the default \Illuminate\Contracts\Debug\ExceptionHandler::renderForConsole method:

screenshot 2017-12-17 22 31 30

Otherwise Collision will render the exception details itself giving an detailed explanation of the problem:

screenshot 2017-12-17 22 57 56

If someone needs details about package or about how it works behind the scenes please don't hesitate to ask.

@GrahamCampbell
Copy link
Member

Thanks, but I don't think we should include a third party library like this tbh.

@timacdonald
Copy link
Member

timacdonald commented Dec 18, 2017

As this package is "Whoops for the console" I don't see a huge problem (from my cosy little bubble of working with Laravel on small projects over here) with including "Whoops for Http" and "Whoops for console". If anything it makes things more consistent as now we see exceptions in the console like we do in the browser. Otherwise you could just argue that "Whoops" should not be included - but we all love that.

I have just started using this and it's awesome. Definitely going to be installing in any future project.

@ankurk91
Copy link
Contributor

Laravel 5.5 supports php v7.0

laravel/composer.json

Lines 7 to 8 in 3f0e742

"require": {
"php": ">=7.0.0",

While collision requires php v7.1

https://github.com/nunomaduro/collision/blob/d4b71295d47d269f385f6f4355e16eeb07a66eff/composer.json#L16-L20

@ElfSundae
Copy link

@ankurk91
Copy link
Contributor

But GitHub indicates that target branch is master not develop.

screenshot from 2017-12-18 10 35 21

@ElfSundae
Copy link

@ankurk91 Oh, sorry, you're right.
@nunomaduro L5.6 is on develop branch, you may change the base branch to develop

@GrahamCampbell GrahamCampbell changed the base branch from master to develop December 18, 2017 11:29
@GrahamCampbell GrahamCampbell changed the base branch from develop to master December 18, 2017 11:30
@GrahamCampbell GrahamCampbell changed the base branch from master to develop December 18, 2017 11:31
@taylorotwell taylorotwell merged commit f5ea753 into laravel:develop Dec 19, 2017
@taylorotwell
Copy link
Member

Seems pretty nice to me. Thanks

@taylorotwell
Copy link
Member

@nunomaduro one thing that I think would be nice is to disable collision when PHPUnit is running. Do you have any thoughts on that?

@nunomaduro
Copy link
Member Author

@taylorotwell You are right! I just tagged v1.1.17 where Collision is disabled on testing environment.

nunomaduro/collision@312f56c

@taylorotwell
Copy link
Member

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants