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

[6.x] Use BindingResolutionException to signal problem with container resolution #32349

Merged
merged 1 commit into from
Apr 13, 2020

Conversation

mfn
Copy link
Contributor

@mfn mfn commented Apr 12, 2020

Same as #32348 but targeting L6

As part of static analysis sometimes abstracts get resolved which aren't
resolvable => that's totally fine but it would help to use a common
exception to signal this.

I figured BindingResolutionException would be the correct one. For example,
it's caught in cases like \Illuminate\Foundation\Exceptions\Handler::whoopsHandler
and IMHO this would also benefit from a streamlined exception.

…ution

As part of static analysis sometimes abstracts get resolved which aren't
resolvable => that's totally fine but it would help to use a common
exception to signal this.

I figured BindingResolutionException would be the correct one. For example,
it's caught in cases like `\Illuminate\Foundation\Exceptions\Handler::whoopsHandler`
and IMHO this would also benefit from a streamlined exception.
@GrahamCampbell
Copy link
Member

Could you please send the same PR to Lumen 7 please (not Lumen 6 since LTS only applied to laravel/framework).

@mfn
Copy link
Contributor Author

mfn commented Apr 13, 2020

Could you please send the same PR to Lumen 7 please (not Lumen 6 since LTS only applied to laravel/framework).

Aye, see laravel/lumen-framework#1065 !

@taylorotwell taylorotwell merged commit 0eb3787 into laravel:6.x Apr 13, 2020
@mfn mfn deleted the mfn-binding branch April 13, 2020 18:28
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.

3 participants