search

DylanPiercey / local-devices

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

Switch to using an object for the arguments and accept a new `arpPath` argument #59

Closed timrogers closed 2 years ago

timrogers commented 2 years ago

This updates the API of the findLocalDevices function to allow you to optionally specify the path of the arp executable using the arpPath option, rather than assuming that it is available in the PATH.

As part of this change, I make a breaking change to the API of the function, switching from positional arguments to a keyed object. This makes the API more friendly and discoverable as the number of options grows, and means you don't have to use nulls to say "I don't want to set this option".

It also corrects the findLocalDevices function type to reflect the skipNameResolution argument which is already accepted.

DylanPiercey commented 2 years ago

@timrogers i think this looks good. Could you update the docs?

timrogers commented 2 years ago

@DylanPiercey 👋🏻 Hope you're having a good weekend! Any chance we can get this merged?

timrogers commented 2 years ago

@DylanPiercey 👋🏻 Just a reminder about this PR when you have a second!

DylanPiercey commented 2 years ago

Hey @timrogers @natterstefan! I'm having a hard time maintaining this project since I haven't actively used it in years and I don't have a lot of confidence in its test suite (eg we're not testing against windows).

Wondering if either of you would like to step in as a maintainer on GitHub (I know @natterstefan already is) with npm publish access also?

timrogers commented 2 years ago

I would be happy to take it on. I don't realistically have huge amounts of time to drive the project forward, but I can at least steward it and review things.

natterstefan commented 2 years ago

Hey @DylanPiercey, Hey @timrogers,

Wondering if either of you would like to step in as a maintainer on GitHub (I know @natterstefan already is) with npm publish access also?

I'm sorry I missed lots of PRs too. Additionally, I was not exactly sure how we proceed with PRs (do we both need to accept it or just one of us) and how we proceed with releases after merges. I am happy to step in as a more active maintainer (together with @timrogers if he wants) proceeding more actively PRs etc. up to a release on npm.

I don't have a lot of confidence in its test suite (eg we're not testing against windows).

Me too with the windows part, but I could set up at least GitHub actions for Linux, macOS, and Windows and execute the command in a native environment.

timrogers commented 2 years ago

@DylanPiercey Ready for re-review!

DylanPiercey commented 2 years ago

Looks good, thanks!

DylanPiercey commented 2 years ago

Published as 4.0.0

natterstefan commented 2 years ago

Thank you, @DylanPiercey.