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

Catch \Throwable#18813

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:2.7fromfprochazka:2.7-php7-throwable
May 23, 2016
Merged

Catch \Throwable#18813

fabpot merged 1 commit intosymfony:2.7fromfprochazka:2.7-php7-throwable
May 23, 2016

Conversation

@fprochazka
Copy link
Contributor

@fprochazkafprochazka commentedMay 19, 2016•
edited
Loading

QA
Branch?2.7, 2.8, 3.0
Bug fix?no
New feature?no
BC breaks?no
Deprecations?no
Tests pass?Yes
Fixed ticketsn/a
LicenseMIT
Doc PRn/a

Related#18765,#15949
Depends on#18812

@nicolas-grekas
Copy link
Member

👍

@xabbuh
Copy link
Member

👍

@fabpot
Copy link
Member

Thank you@fprochazka.

@fabpotfabpot merged commit103526b intosymfony:2.7May 23, 2016
fabpot added a commit that referenced this pull requestMay 23, 2016
This PR was merged into the 2.7 branch.Discussion----------Catch \Throwable| Q             | A| ------------- | ---| Branch?       | 2.7, 2.8, 3.0| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | Yes| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aRelated#18765,#15949Depends on#18812Commits-------103526b Catch \Throwable
fabpot added a commit that referenced this pull requestMay 23, 2016
This PR was merged into the 2.8 branch.Discussion----------Catch \Throwable| Q             | A| ------------- | ---| Branch?       | 2.8, 3.0| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | Mostly!| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aThe first commit is based on#15949Depends on#18813,#18812----I'm new to symfony, so I'm not sure where are all the places where it makes sense to actually catch the throwable and where not. I added most places that seemed logical and when I wasn't sure, I added it anyway. I'm hoping you guys (and girls?) can point out the places where the catch should not be added, I'll fix it and then I can create several PR's for the older branches. A lot of this IMHO should go also to 3.0.Commits-------de671f4 Catch \Throwable
symfony-splitter pushed a commit to symfony/dependency-injection that referenced this pull requestMay 23, 2016
This PR was merged into the 2.8 branch.Discussion----------Catch \Throwable| Q             | A| ------------- | ---| Branch?       | 2.8, 3.0| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | Mostly!| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aThe first commit is based onsymfony/symfony#15949Depends onsymfony/symfony#18813,symfony/symfony#18812----I'm new to symfony, so I'm not sure where are all the places where it makes sense to actually catch the throwable and where not. I added most places that seemed logical and when I wasn't sure, I added it anyway. I'm hoping you guys (and girls?) can point out the places where the catch should not be added, I'll fix it and then I can create several PR's for the older branches. A lot of this IMHO should go also to 3.0.Commits-------de671f4 Catch \Throwable
@fprochazkafprochazka deleted the 2.7-php7-throwable branchMay 23, 2016 13:32
@fabpotfabpot mentioned this pull requestMay 26, 2016
@patrick-mcdougle
Copy link
Contributor

patrick-mcdougle commentedMay 31, 2016•
edited
Loading

According to the PHP documentation:

PHP classes cannot implement the Throwable interface directly, and must instead extend Exception.

Are we sure this change is needed? Additionally, what does this do when php < 7?

@fprochazka
Copy link
ContributorAuthor

fprochazka commentedJun 5, 2016•
edited
Loading

Are we sure this change is needed?

Yes it is. Theese cases would normally be handled usingfinally, but since this branch supports PHP 5.3, it has to be done like this.

Additionally, what does this do when php < 7?

it is ignored

@patrick-mcdougle
Copy link
Contributor

I guess what I mean is that everything would be caught by the abovecatch(\Exception) block since all \Throwable MUST extend \Excpetion.

@xabbuh
Copy link
Member

@patrick-mcdougle TheError class (introduced in PHP 7) implementsThrowable but does not extend theException class.

@patrick-mcdougle
Copy link
Contributor

Thanks for the explanation@xabbuh !

This was referencedJun 6, 2016
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@fprochazka@nicolas-grekas@xabbuh@fabpot@patrick-mcdougle@javiereguiluz@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp