-
Notifications
You must be signed in to change notification settings - Fork 47
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
Fixes issue # 54: IoManager._get_responses_windows
mangles token when reading from stdout
#55
Conversation
…esponses_windows.
Hey, sorry for the ridiculously delayed reply, and thanks for taking the time to contribute. Would you be interested in updating this to fix the merge conflict and make the CI tests pass? I will give you a quick review (within a few days) if you choose to. |
I like that stack overflow answer. Getting rid of OS-specific stuff would be nice. |
As the patch was not updated, I can take this and rebased it against |
Sorry for the super late reply. I'd appreciate it if anyone could rebase this as I switched jobs and no longer have access to a Windows system. |
Original patch by Leonardo Pereira Santos (see cs01#55); updated by Marco Barisione.
I made a separate PR (#76) to update this (as I cannot push to somebody else's repository). Tests don't pass on Unix and I'm reluctant to modify the code as I cannot test it on Windows. |
Original patch by Leonardo Pereira Santos (see cs01#55); updated by Marco Barisione.
SUPERSEDED BY #76
CHANGELOG.md
Summary of changes
Fixes issue #54
Removed the
make_non_blocking
function. Refactored the_get_responses_windows
method to use separate threads and queues to make reading non-blocking. solution based on this StackOverflow answerTest plan
Executed unit tests: