Skip to content
This repository has been archived by the owner on Dec 17, 2019. It is now read-only.

Kill reactor properly #12

Merged
merged 3 commits into from
Dec 4, 2015
Merged

Kill reactor properly #12

merged 3 commits into from
Dec 4, 2015

Conversation

rojepp
Copy link
Contributor

@rojepp rojepp commented Dec 3, 2015

Fixes #10, tested on OSX only.

Fixes #10, tested on OSX only.
@Krzysztof-Cieslak
Copy link
Member

I'm not sure if it's enough - in Ionide we do something like that to kill processes on Windows https://github.com/ionide/ionide-vscode-fake/blob/master/src/fake.fs#L51-L55

@rojepp
Copy link
Contributor Author

rojepp commented Dec 3, 2015

Thanks! A combination of the two worked.

@rojepp
Copy link
Contributor Author

rojepp commented Dec 3, 2015

Probably reactor should also be killed when code exits?

@Krzysztof-Cieslak
Copy link
Member

I think child processes are killed when node applications exists.

@rojepp
Copy link
Contributor Author

rojepp commented Dec 3, 2015

Reactor process still lives on Windows after I quit code. I'll check OSX tomorrow.

@Krzysztof-Cieslak
Copy link
Member

Looks like it's problem in VS Code indeed, microsoft/vscode-go#26

@rojepp
Copy link
Contributor Author

rojepp commented Dec 4, 2015

We can just call a silent version stopReactor on process exit? I'll do that in a few hours.

@Krzysztof-Cieslak
Copy link
Member

It was discussed in issue I've linked, looks like there are some problems with that.

@rojepp
Copy link
Contributor Author

rojepp commented Dec 4, 2015

We could probably implement some hack to do it, but I'd prefer waiting for vscode support for deactivation. I'd say this PR is ready to merge.

@Krzysztof-Cieslak
Copy link
Member

👍

rojepp added a commit that referenced this pull request Dec 4, 2015
@rojepp rojepp merged commit db2ae80 into elm-tooling:master Dec 4, 2015
@rojepp rojepp deleted the kill_reactor branch December 4, 2015 12:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants