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

terminate() function #79

Merged
merged 5 commits into from
Feb 13, 2018
Merged

terminate() function #79

merged 5 commits into from
Feb 13, 2018

Conversation

brucedjones
Copy link
Collaborator

Allows you to terminate a running python script.

Terminates the child process and calls the end callback
@Almenon
Copy link
Collaborator

Almenon commented Jan 22, 2018

I like it!

But kill alone may not suffice for exiting the script. There should be a paramater in the function that lets you send a kill signal like SIGKILL

Example: https://github.com/Almenon/AREPL-backend/blob/master/index.js#L78

Though if you kill via signal you might also want to fix the close event functionality #94

@the-freshlord
Copy link

Could this be merged anytime soon? I actually need this functionality in a project that I am working on. 😀

@brucedjones
Copy link
Collaborator Author

So I added the ability to send a kill signal. One thing that was not clear to me, was why terminate should occur if both stderr and stdout are closed, the main thing we care about is the process itself ending. So I removed that functionality as it seemed like unnecessary extra complication.

@brucedjones
Copy link
Collaborator Author

So in fixing the bugs I put the ending on stderr and stdout back in. I figured it better to be consistent with previous behavior.

@brucedjones
Copy link
Collaborator Author

@extrabacon any objections to this pull request?

@extrabacon
Copy link
Owner

Not at all, I just want to make sure I can publish shortly after merging. So sorry, too much work means I don't have time to properly handle this project.

@Almenon
Copy link
Collaborator

Almenon commented Feb 10, 2018

@extrabacon maybe someone could be added as a maintainer? I wouldn't mind helping out with the project. Or maybe @brucedjones?

@extrabacon
Copy link
Owner

@brucedjones + @Almenon: you are now collaborators on the repo
@brucedjones: you can also publish to NPM

@Almenon
Copy link
Collaborator

Almenon commented Feb 13, 2018

Thanks extrabacon. I guess I'll start out by taking a look at the unit tests. @brucedjones, want to merge in your pull request?

@Almenon
Copy link
Collaborator

Almenon commented Feb 11, 2023

@brucedjones I'm looking for a new maintainer to take on my duties, are you still up for it?

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.

4 participants