-
Notifications
You must be signed in to change notification settings - Fork 27
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
Fixed win32 parser for better windows support #9
Fixed win32 parser for better windows support #9
Conversation
Would you be able to update the tests to add win32 as well? |
Ops sorry, now tests are ok 👍 |
@DylanPiercey looks good to me, what do you think? |
I was hoping to get a test that covered the change, but if you both have tested this the. It’s fine by me 🙂 |
The test is updated to cover this pull, tell me if I missed something :) |
@DylanPiercey I was not yet able to test it on windows, but travis says it's still working. @LucaSoldi Where did you update the test? I can't see any changed test file in your PR. Have you tried it on your win? |
Hi @DylanPiercey, what do you suggest what we do with this PR? Merge and release a new version? |
@natterstefan I'm fine with it being merged. Again I'd like to see a proper test, but it's a bit tricky I suppose to fake with the CI. |
@DylanPiercey Good idea with the CI. Let's see how we need to handle the mocked IPs in travis differently in the windows case then: Lines 26 to 31 in 4403a8c
|
Windows 10 return string from child_process that doesn't contain "\t" characters.
I tested this split function on a Win7/10 and it works like a charm :)