Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Add PHPDbg support to HTTP components#26749
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
lyrixx left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for your PR.
I left a minor comment.
Did you check for all references toPHP_SAPI ?
| if (function_exists('fastcgi_finish_request')) { | ||
| fastcgi_finish_request(); | ||
| }elseif ('cli' !==PHP_SAPI) { | ||
| }elseif (!in_array(PHP_SAPI,array('cli','phpdbg'))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could you update your code with} elseif (!\in_array(PHP_SAPI, array('cli', 'phpdbg'), true)) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Done.
hkdobrev commentedApr 2, 2018
@lyrixx Thanks for the quick review! I did check every usage of |
hkdobrev commentedApr 2, 2018
I've just unified the code style in all checks. |
hkdobrev commentedApr 2, 2018
Important consideration is these would need to be re-checked in 2.8, 3.4, 4.0 etc. |
fabpot commentedApr 3, 2018
Thank you@hkdobrev. |
This PR was squashed before being merged into the 2.7 branch (closes#26749).Discussion----------Add PHPDbg support to HTTP components| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |This is a follow-up to#26677.Turns out there aren't that many remaining instances of `PHP_SAPI` checks without considering `phpdbg` where it's needed.Commits-------60dd79c Add PHPDbg support to HTTP components
This is a follow-up to#26677.
Turns out there aren't that many remaining instances of
PHP_SAPIchecks without consideringphpdbgwhere it's needed.