Skip to content

[Process] remove deprecated features #22836

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

Merged
merged 1 commit into from
Jun 28, 2017

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented May 21, 2017

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

@xabbuh xabbuh added this to the 3.4 milestone May 21, 2017
@xabbuh xabbuh modified the milestones: 4.0, 3.4 May 21, 2017
@nicolas-grekas
Copy link
Member

tests are failing :)

@xabbuh xabbuh force-pushed the process-remove-deprecated-features branch 4 times, most recently from 89f9b00 to 19ff37a Compare May 22, 2017 07:57
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

Tests still do not pass apparently.

@@ -1538,7 +1393,7 @@ private function close()
if ($this->processInformation['signaled'] && 0 < $this->processInformation['termsig']) {
// if process has been signaled, no exitcode but a valid termsig, apply Unix convention
$this->exitcode = 128 + $this->processInformation['termsig'];
} elseif ($this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
} else {
Copy link
Member

Choose a reason for hiding this comment

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

extra space here

@xabbuh xabbuh force-pushed the process-remove-deprecated-features branch from 19ff37a to 5e582b7 Compare June 13, 2017 07:57
@ogizanagi ogizanagi mentioned this pull request Jun 21, 2017
* environment variables will always be inherited
* the `ProcessUtils::escapeArgument()` method has been removed
* the `inheritEnvironmentVariables()` and `setOption()` options of the
`ProcessBuilder` class have been removed
Copy link
Member

Choose a reason for hiding this comment

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

Wrong changelog entry. The whole class is gone

fabpot added a commit that referenced this pull request Jun 21, 2017
This PR was merged into the 4.0-dev branch.

Discussion
----------

[DI] Uncomment code

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see comment below -->
| Bug fix?      | no
| New feature?  | no <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | N/A <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

I was about fixing all [other occurrences of commented code and `func_get/num_arg(s)`](https://github.com/symfony/symfony/search?utf8=%E2%9C%93&q=func_num_args&type=) as BC layer usages at once, but the following PRs will already cover it:

- HttpFoundation: #22863
- Serializer: #23241
- Process: #22836
- DotEnv: #23242

So it only remains DI ones.

Commits
-------

03f33b5 [DI] Uncomment code
@xabbuh xabbuh force-pushed the process-remove-deprecated-features branch from 5e582b7 to e4ec8e9 Compare June 26, 2017 07:25
@xabbuh
Copy link
Member Author

xabbuh commented Jun 26, 2017

I have removed the sigchild compatibility part as I did not manage to make it work properly. If someone wants to give it a try, you can just base your work on this PR or create a new one.

@fabpot
Copy link
Member

fabpot commented Jun 28, 2017

Let's merge this one as is then.

@fabpot
Copy link
Member

fabpot commented Jun 28, 2017

Thank you @xabbuh.

@fabpot fabpot merged commit e4ec8e9 into symfony:master Jun 28, 2017
fabpot added a commit that referenced this pull request Jun 28, 2017
This PR was merged into the 4.0-dev branch.

Discussion
----------

[Process] remove deprecated features

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

Commits
-------

e4ec8e9 [Process] remove deprecated features
@xabbuh xabbuh deleted the process-remove-deprecated-features branch June 28, 2017 07:40
nicolas-grekas added a commit that referenced this pull request Jul 12, 2017
…maid)

This PR was merged into the 4.0-dev branch.

Discussion
----------

[Process] Remove enhanced sigchild compatibility

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #23376 (comment), #22836 (comment)
| License       | MIT
| Doc PR        | /

In 4.0, enhanced sigchild compatibility is always enabled.

Commits
-------

1aa7d68 Remove enhance sigchild compatibility
symfony-splitter pushed a commit to symfony/process that referenced this pull request Jul 12, 2017
…maid)

This PR was merged into the 4.0-dev branch.

Discussion
----------

[Process] Remove enhanced sigchild compatibility

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony#23376 (comment), symfony/symfony#22836 (comment)
| License       | MIT
| Doc PR        | /

In 4.0, enhanced sigchild compatibility is always enabled.

Commits
-------

1aa7d68 Remove enhance sigchild compatibility
@fabpot fabpot mentioned this pull request Oct 19, 2017
symfony-splitter pushed a commit to symfony/process that referenced this pull request Jul 18, 2019
…es() (ogizanagi)

This PR was merged into the 4.4 branch.

Discussion
----------

[Process] Deprecate Process::inheritEnvironmentVariables()

| Q             | A
| ------------- | ---
| Branch?       | 4.4 <!-- see below -->
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | N/A   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

IIUC, this method was kept as a BC layer from 3.4 to 4.0 to switch to the "inherit env vars" behavior, inciting developers to opt-in in 3.4. Since 4.0, env vars are always inherited, and this method doesn't allow to opt-out. So, time to remove it?

---

refs:

- symfony/symfony#21470
- symfony/symfony#22836

Commits
-------

af9bad31c6 [Process] Deprecate Process::inheritEnvironmentVariables()
nicolas-grekas added a commit that referenced this pull request Jul 18, 2019
…es() (ogizanagi)

This PR was merged into the 4.4 branch.

Discussion
----------

[Process] Deprecate Process::inheritEnvironmentVariables()

| Q             | A
| ------------- | ---
| Branch?       | 4.4 <!-- see below -->
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | N/A   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

IIUC, this method was kept as a BC layer from 3.4 to 4.0 to switch to the "inherit env vars" behavior, inciting developers to opt-in in 3.4. Since 4.0, env vars are always inherited, and this method doesn't allow to opt-out. So, time to remove it?

---

refs:

- #21470
- #22836

Commits
-------

af9bad3 [Process] Deprecate Process::inheritEnvironmentVariables()
symfony-splitter pushed a commit to symfony/web-server-bundle that referenced this pull request Jul 18, 2019
…es() (ogizanagi)

This PR was merged into the 4.4 branch.

Discussion
----------

[Process] Deprecate Process::inheritEnvironmentVariables()

| Q             | A
| ------------- | ---
| Branch?       | 4.4 <!-- see below -->
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | N/A   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

IIUC, this method was kept as a BC layer from 3.4 to 4.0 to switch to the "inherit env vars" behavior, inciting developers to opt-in in 3.4. Since 4.0, env vars are always inherited, and this method doesn't allow to opt-out. So, time to remove it?

---

refs:

- symfony/symfony#21470
- symfony/symfony#22836

Commits
-------

af9bad31c6 [Process] Deprecate Process::inheritEnvironmentVariables()
symfony-splitter pushed a commit to symfony/var-dumper that referenced this pull request Jul 18, 2019
…es() (ogizanagi)

This PR was merged into the 4.4 branch.

Discussion
----------

[Process] Deprecate Process::inheritEnvironmentVariables()

| Q             | A
| ------------- | ---
| Branch?       | 4.4 <!-- see below -->
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | N/A   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

IIUC, this method was kept as a BC layer from 3.4 to 4.0 to switch to the "inherit env vars" behavior, inciting developers to opt-in in 3.4. Since 4.0, env vars are always inherited, and this method doesn't allow to opt-out. So, time to remove it?

---

refs:

- symfony/symfony#21470
- symfony/symfony#22836

Commits
-------

af9bad31c6 [Process] Deprecate Process::inheritEnvironmentVariables()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants