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

F/process spawn #984

Closed
wants to merge 5 commits into from
Closed

Conversation

technorama
Copy link
Contributor

Add Process#spawn (almost identical to ruby version with some features not yet implemented).
Add some documentation.
Better error checking in a few places and less chance to leak file descriptors.

@jhass
Copy link
Member

jhass commented Jul 15, 2015

@technorama technorama force-pushed the f/process_spawn branch 5 times, most recently from 7275dce to 125a498 Compare August 13, 2015 15:32
Refactor Process#run
  code simplified
  now accepts stderr as an argument

Add Process::Status#close
  closes any pipes to the child and waits for the process to exit

Add minimal documentation and additional specs
@asterite
Copy link
Member

@technorama "We" finally implemented this in bbf2036

I say "we" because we used your code a lot. We were very happy to find out you did exactly what we had in mind. The only small issue we had is that we didn't want to have more than one way to execute a process (spawn, popen, run), but just a single name: run.

I briefly documented the methods, I guess we would need to add more docs and examples showing many usages, because the API is very flexible. We also didn't include the chdir, umask and env options, but we'll do it soon.

Thank you so much for working on this!! Once we saw your implementation and think/combined it with what we had in mind, doing it was a breeze! We also hope you like the simplification we made :-)

@asterite asterite closed this Aug 14, 2015
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.

3 participants