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

[Process] Added support for non-blocking process control #3681

Merged
merged 2 commits into from Mar 23, 2012
Merged

[Process] Added support for non-blocking process control #3681

merged 2 commits into from Mar 23, 2012

Conversation

mxrth
Copy link

@mxrth mxrth commented Mar 23, 2012

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes Build Status
Fixes the following tickets: #1049
Todo: -

Added methods to control long running processes (as described in issue #1049) to the Process class:

  • A non blocking start method to startup a process and return
    immediately
  • A blocking waitForTermination method to wait for the processes
    termination
  • A stop method to stop a process started with start
    All status-getters like getOutput were changed to return real-time data

Added methods to control long running processes to the Process class:
 - A non blocking start method to startup a process and return
   immediately
 - A blocking waitForTermination method to wait for the processes
   termination
 - A stop method to stop a process started with start
All status-getters like getOutput were changed to return real-time data

/**
* Starts the process and returns after sending the stdin.
* This method blocks until all stdin data is sent to the process then it returns while the pricess runs in the background.
Copy link
Member

Choose a reason for hiding this comment

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

typo here: pricess instead of process

@Seldaek
Copy link
Member

Seldaek commented Mar 23, 2012

Overall this seems like a good improvement. I didn't check the code in detail though.


if (false === $n) {
break;
} elseif ($n === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

simply use if here as the previous if breaks the loop.

And it should be if (0 === $n) {

@mxrth
Copy link
Author

mxrth commented Mar 23, 2012

@stof @Seldaek thanks, fixed

fabpot added a commit that referenced this pull request Mar 23, 2012
Commits
-------

f9f51a5 fixed cs
af65673 [Process] Added support for non-blocking process control Added methods to control long running processes to the Process class:  - A non blocking start method to startup a process and return    immediately  - A blocking waitForTermination method to wait for the processes    termination  - A stop method to stop a process started with start All status-getters like getOutput were changed to return real-time data

Discussion
----------

[Process] Added support for non-blocking process control

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes [![Build Status](https://secure.travis-ci.org/drak3/symfony.png?branch=process-control)](http://travis-ci.org/drak3/symfony)
Fixes the following tickets: #1049
Todo: -

Added methods to control long running processes (as described in issue #1049) to the Process class:
 - A non blocking start method to startup a process and return
   immediately
 - A blocking waitForTermination method to wait for the processes
   termination
 - A stop method to stop a process started with start
All status-getters like getOutput were changed to return real-time data

---------------------------------------------------------------------------

by Seldaek at 2012-03-23T10:52:30Z

Overall this seems like a good improvement. I didn't check the code in detail though.

---------------------------------------------------------------------------

by drak3 at 2012-03-23T11:21:45Z

@stof @Seldaek thanks, fixed
@fabpot fabpot merged commit f9f51a5 into symfony:master Mar 23, 2012
@b00gizm
Copy link

b00gizm commented Mar 23, 2012

Awesome! Thanks a lot :)

Tobion added a commit that referenced this pull request Jan 19, 2016
This PR was merged into the 2.3 branch.

Discussion
----------

[Process] Remove a misleading comment

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

The described behaviour was never implemented as part of #3681.

Commits
-------

71f4a32 [Process] Remove a misleading comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants