search

DylanPiercey / local-devices

🔮 Find devices connected to the current local network.
MIT License
139 stars 27 forks source link

Fixed win32 parser for better windows support #9

Closed LucaSoldi closed 5 years ago

LucaSoldi commented 5 years ago

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 :)

DylanPiercey commented 5 years ago

Would you be able to update the tests to add win32 as well?

LucaSoldi commented 5 years ago

Ops sorry, now tests are ok 👍

natterstefan commented 5 years ago

@DylanPiercey looks good to me, what do you think?

DylanPiercey commented 5 years ago

I was hoping to get a test that covered the change, but if you both have tested this the. It’s fine by me 🙂

LucaSoldi commented 5 years ago

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 :)

natterstefan commented 5 years ago

@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?

natterstefan commented 5 years ago

Hi @DylanPiercey, what do you suggest what we do with this PR? Merge and release a new version?

DylanPiercey commented 5 years ago

@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.

natterstefan commented 5 years ago

@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: https://github.com/DylanPiercey/local-devices/blob/4403a8cf431e3f7dafa4d0f4ba59126c836fdcd4/jest-setup.js#L26-L31.