[PhpUnitBridge] Provide debug_backtrace with proper args #28506
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
greg0ire
changed the title
greg0ire
force-pushed
the
provide_debug_backtrace_with_proper_args
branch
2 times, most recently
from
3ffa6d1
to
cfe6ae1
Compare
greg0ire
commented
Sep 18, 2018
| @@ -72,7 +72,7 @@ public static function register($mode = 0) | |||
| } | |||
|
|
|||
| $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 | |||
There was a problem hiding this comment.
Note that DEBUG_BACKTRACE_PROVIDE_OBJECT is the default value for this argument.
There was a problem hiding this comment.
so let's remove it entirely instead
|
why do we care? we don't use strict_types (and we don't want to.) |
|
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 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 symfony#18272, but I do not understand why it was simplified, nor why DEBUG_BACKTRACE_IGNORE_ARGS was there at that time.
greg0ire
force-pushed
the
provide_debug_backtrace_with_proper_args
branch
from
cfe6ae1
to
0d826ae
Compare
|
Thank you @greg0ire. |
nicolas-grekas
added a commit
that referenced
this pull request
Sep 20, 2018
… (greg0ire) This PR was merged into the 2.8 branch. Discussion ---------- [PhpUnitBridge] Provide debug_backtrace with proper args 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. | 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/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. Commits ------- 0d826ae Provide debug_backtrace with proper args
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.