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

[PhpUnitBridge] Provide debug_backtrace with proper args#28506

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

Conversation

@greg0ire
Copy link
Contributor

@greg0iregreg0ire commentedSep 18, 2018
edited by nicolas-grekas
Loading

This would fail if we were using strict mode with php 7, because true is
only a valid argument for php < 5.3.6.
This was changed from PHP_VERSION_ID >= 50400 ?
DEBUG_BACKTRACE_IGNORE_ARGS | DEBUG_BACKTRACE_PROVIDE_OBJECT : true in
#18272, but I do not understand why it was simlified, nor why
DEBUG_BACKTRACE_IGNORE_ARGS was there at that time.

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

Not sure if this qualifies as a bugfix. If not, should I simplify the ternary and target master? My fear is that having the code diverge too much will make it harder to merge subsequent PRs. I know this looks small, but I'm kind of preparing a big PR on the bridge and I'd rather have it smaller and easier to understand by moving everything I can with small patches like this one.

@greg0iregreg0ire changed the titleProivde debug_backtrace with proper args[PhpUnitBridge] Provide debug_backtrace with proper argsSep 18, 2018
@greg0iregreg0ireforce-pushed theprovide_debug_backtrace_with_proper_args branch 2 times, most recently from3ffa6d1 tocfe6ae1CompareSeptember 18, 2018 21:13

$mode =$getMode();
$trace =debug_backtrace(true);
$trace =debug_backtrace(\defined(DEBUG_BACKTRACE_PROVIDE_OBJECT) ?DEBUG_BACKTRACE_PROVIDE_OBJECT :true);// simplify when dropping php < 5.3.6
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Note thatDEBUG_BACKTRACE_PROVIDE_OBJECT is the default value for this argument.

Choose a reason for hiding this comment

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

so let's remove it entirely instead

greg0ire reacted with thumbs up emoji
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedSep 19, 2018
edited
Loading

why do we care? we don't use strict_types (and we don't want to.)

@nicolas-grekasnicolas-grekas added this to the2.8 milestoneSep 19, 2018
@stof
Copy link
Member

this is not a bugfix as there is no bug. This file is not in strict mode, and cannot become in strict mode unless you edit the source code yourselves (in which case,you are introducing a bug)

This would fail if we were using strict mode with php 7, because true isonly a valid argument for php < 5.3.6.This was changed from PHP_VERSION_ID >= 50400 ?DEBUG_BACKTRACE_IGNORE_ARGS | DEBUG_BACKTRACE_PROVIDE_OBJECT : true insymfony#18272, but I do not understand why it was simplified, nor whyDEBUG_BACKTRACE_IGNORE_ARGS was there at that time.
@greg0iregreg0ireforce-pushed theprovide_debug_backtrace_with_proper_args branch fromcfe6ae1 to0d826aeCompareSeptember 19, 2018 20:35
@nicolas-grekas
Copy link
Member

Thank you@greg0ire.

@nicolas-grekasnicolas-grekas merged commit0d826ae intosymfony:2.8Sep 20, 2018
nicolas-grekas added a commit that referenced this pull requestSep 20, 2018
… (greg0ire)This PR was merged into the 2.8 branch.Discussion----------[PhpUnitBridge] Provide debug_backtrace with proper argsThis would fail if we were using strict mode with php 7, because true isonly a valid argument for php < 5.3.6.This was changed from PHP_VERSION_ID >= 50400 ?DEBUG_BACKTRACE_IGNORE_ARGS | DEBUG_BACKTRACE_PROVIDE_OBJECT : true in#18272, but I do not understand why it was simlified, nor whyDEBUG_BACKTRACE_IGNORE_ARGS was there at that time.| Q             | A| ------------- | ---| Branch?       | 2.8| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aNot sure if this qualifies as a bugfix. If not, should I simplify the ternary and target master? My fear is that having the code diverge too much will make it harder to merge subsequent PRs. I know this looks small, but I'm kind of preparing a big PR on the bridge and I'd rather have it smaller and easier to understand by moving everything I can with small patches like this one.Commits-------0d826ae Provide debug_backtrace with proper args
@greg0iregreg0ire deleted the provide_debug_backtrace_with_proper_args branchSeptember 20, 2018 11:14
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

Assignees

No one assigned

Projects

None yet

Milestone

2.8

Development

Successfully merging this pull request may close these issues.

4 participants

@greg0ire@nicolas-grekas@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp