Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.3][Process] fix Proccess run with pts enabled #16510

Closed
wants to merge 6 commits into from

Conversation

Projects
None yet
3 participants
@ewgRa
Copy link
Contributor

ewgRa commented Nov 10, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #12643
License MIT
Doc PR

ewgRa added some commits Nov 10, 2015

Revert "fix cs"
This reverts commit c4cf1c6.
@ewgRa

This comment has been minimized.

Copy link
Contributor Author

ewgRa commented Nov 10, 2015

When fabbot.io patch is applied, there is a conflicts, I think better to merge it without conflicts.

@ewgRa

This comment has been minimized.

Copy link
Contributor Author

ewgRa commented Nov 10, 2015

Test failure seems not related to this PR

if (!$this->useFileHandles && $this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
// Workaround for the bug, when PTS functionality is enabled.
// @see : https://github.com/symfony/symfony/issues/12643

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 10, 2015

Member

no need to ref this one in the code

if (!$this->useFileHandles && $this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
// Workaround for the bug, when PTS functionality is enabled.
// @see : https://github.com/symfony/symfony/issues/12643
// @see : https://github.com/php/php-src/pull/1588

This comment has been minimized.

Copy link
@nicolas-grekas
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Nov 10, 2015

@ewgRa could you please fabbot's issues also (even if unrelated to your change)?

ewgRa added some commits Nov 10, 2015

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Nov 10, 2015

See #12643 (comment) for some background on why this works

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Nov 10, 2015

👍 (after rebasing)

@ewgRa

This comment has been minimized.

Copy link
Contributor Author

ewgRa commented Nov 10, 2015

done, anything else?

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Nov 10, 2015

Thank you @ewgRa.

nicolas-grekas added a commit that referenced this pull request Nov 10, 2015

bug #16510 [2.3][Process] fix Proccess run with pts enabled (ewgRa)
This PR was squashed before being merged into the 2.3 branch (closes #16510).

Discussion
----------

[2.3][Process] fix Proccess run with pts enabled

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #12643
| License       | MIT
| Doc PR        |

Commits
-------

9cf90fb [2.3][Process] fix Proccess run with pts enabled
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Nov 10, 2015

Closed via ab3c8f8

@ewgRa ewgRa deleted the ewgRa:issue-12643 branch Nov 10, 2015

This was referenced Nov 23, 2015

This was referenced Nov 30, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.