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

[Debug][ErrorHandler] Do not use the php80 polyfill#42223

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
nicolas-grekas merged 1 commit intosymfony:4.4fromnicolas-grekas:eh-nopolyfill
Jul 22, 2021

Conversation

@nicolas-grekas
Copy link
Member

QA
Branch?4.4
Bug fix?no
New feature?no
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

@carsonbotcarsonbot added this to the4.4 milestoneJul 21, 2021
@nicolas-grekasnicolas-grekas changed the title[ErrorHandler] Do not use the php80 polyfill in DebugClassLoader[ErrorHandler] Do not use the php80 polyfillJul 21, 2021
@nicolas-grekasnicolas-grekas changed the title[ErrorHandler] Do not use the php80 polyfill[ErrorHandler][Debug] Do not use the php80 polyfillJul 21, 2021
@stof
Copy link
Member

the usage ofget_debug_type inFatalThrowableError also seems to cause issues (see the Goutte issue referencing that ticket)

@derrabus
Copy link
Member

Out of curiosity: What's the problem with using the polyfill here?

@carsonbotcarsonbot changed the title[ErrorHandler][Debug] Do not use the php80 polyfill[ErrorHandler] Do not use the php80 polyfillJul 21, 2021
@stof
Copy link
Member

@derrabus polyfills rely on autoloading (for the internal class). So this breaks if the polyfill is called inside the autoloading stack (as autoloading the polyfill internal classes will also try to use them then).
For the error handling part, I suspect it is similar (for cases where an error happens during the error handling or during the autoloading of the polyfill internals)

derrabus, nicolas-grekas, and Nyholm reacted with thumbs up emoji

Copy link
Contributor

@TobionTobion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Shouldn't this remove the polyfill-php80 dependency from composer.json then?

@Nyholm
Copy link
Member

Nyholm commentedJul 22, 2021
edited
Loading

I dont see us using any PHP8-only code in the ErrorHandler. I agree with@Tobion

With that change, Im 👍

@nicolas-grekas
Copy link
MemberAuthor

Polyfill now removed (we were using a few calls toget_debug_type(), now removed).

@carsonbotcarsonbot changed the title[ErrorHandler] Do not use the php80 polyfill[Debug][ErrorHandler] Do not use the php80 polyfillJul 22, 2021
@nicolas-grekasnicolas-grekas merged commit8c84365 intosymfony:4.4Jul 22, 2021
@Tobion
Copy link
Contributor

I guess we could revert this in 6.0 as it will require php 8?

@stof
Copy link
Member

stof commentedJul 22, 2021
edited
Loading

@Tobion yes (but without re-adding the polyfill package)

@nicolas-grekasnicolas-grekas deleted the eh-nopolyfill branchJuly 22, 2021 11:09
@nicolas-grekas
Copy link
MemberAuthor

I did revert while merging up.

This was referencedJul 26, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@derrabusderrabusderrabus approved these changes

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

+1 more reviewer

@TobionTobionTobion requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

6 participants

@nicolas-grekas@stof@derrabus@Nyholm@Tobion@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp