Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[HttpKernel] change $previous argument for HttpException to \Throwable#30729

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromsGy1980de:4.2
Mar 30, 2019
Merged

Conversation

@sGy1980de
Copy link
Contributor

QA
Branch?4.2
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#30728
LicenseMIT

This willfix#30728 with the suggested solution to change the signature ofHttpException and all its descendants from\Exception to\Throwable.

@nicolas-grekas
Copy link
Member

Thanks, LGTM. Should target master as it's a new feature.

ro0NL reacted with thumbs up emoji

@sGy1980de
Copy link
ContributorAuthor

@nicolas-grekas
Could you please explain a little, why you consider this a new feature? To me it is a bug or at least unintended behaviour. Furthermore this won't change any existing application out there, so i really don't get the point here.

@nicolas-grekas
Copy link
Member

This never worked as you said, so it wasn't supported. Adding support for something that wasn't possible before is almost the definition of a new feature to me.

@sGy1980de
Copy link
ContributorAuthor

So what's next?
@javiereguiluz marked the corresponding issue as bug. Shall i raise it with targetmaster again or let it like that?

@javiereguiluz
Copy link
Member

@sGy1980de I marked it as a bug because the issue description looked as a bug. But I could be wrong, so please don't think that it's definitive.

@sGy1980de
Copy link
ContributorAuthor

Hey guys - what about the bug vs. feature discussion on this? Is it discussed internally or are all following@nicolas-grekas opinion and i should raise this with target master again?

@fabpot
Copy link
Member

I tend to agree with@nicolas-grekas here.

@fabpot
Copy link
Member

Thank you@sGy1980de.

@fabpotfabpot merged commit15cb475 intosymfony:masterMar 30, 2019
fabpot added a commit that referenced this pull requestMar 30, 2019
…on to \Throwable (sGy1980de)This PR was submitted for the 4.2 branch but it was squashed and merged into the 4.3-dev branch instead (closes#30729).Discussion----------[HttpKernel] change $previous argument for HttpException to \Throwable| Q             | A| ------------- | ---| Branch?       |  4.2| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#30728| License       | MITThis willfix#30728 with the suggested solution to change the signature of `HttpException` and all its descendants from `\Exception` to `\Throwable`.Commits-------15cb475 [HttpKernel] change $previous argument for HttpException to \Throwable
fabpot added a commit that referenced this pull requestMar 30, 2019
This PR was merged into the 4.3-dev branch.Discussion----------Fix some exception previous type hints| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks?    | no     <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->| Fixed tickets | n/a| License       | MIT| Doc PR        | n/afollow-up for#30729Commits-------f92efeb fixed some exception previous type hints
@fabpotfabpot mentioned this pull requestMay 9, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof approved these changes

@dunglasdunglasAwaiting requested review from dunglas

@lyrixxlyrixxAwaiting requested review from lyrixx

@srozesrozeAwaiting requested review from sroze

@xabbuhxabbuhAwaiting requested review from xabbuh

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[HttpKernel] HttpException does not allow \Error as $previous argument

6 participants

@sGy1980de@nicolas-grekas@javiereguiluz@fabpot@stof@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp