search

njh / node-owfs

A node.js client library for the owserver protocol in owfs
https://www.npmjs.com/package/owfs
MIT License
10 stars 4 forks source link

Support owfs message type 'presence' #38

Closed waldbaer closed 5 years ago

waldbaer commented 5 years ago

Useful to check the existence of 1-Wire devices like the DS2411.

If you have any improvements, just let me know. I would be happy if this change gets accepted.

waldbaer commented 5 years ago

Ping?

njh commented 5 years ago

Apologies; I completely forgot about this.

Please can you fix the very minor style problem - that is why the Travis build failed.

waldbaer commented 5 years ago

I fixed the small style-guide violation. sorry didn't see that initially. Travis CI still fails for NPM versions 4 and 5. But this seems not to be related to my change. Looks like an issue in eslint for these NPM versions. Can you have a look on these errors?

njh commented 5 years ago

Yeah, I am bit stuck

  1. I want to support some older versions of node
  2. I want to use the latest versions of eslint / standard.js, to avoid security vulnerabilities
  3. There two seem to be incompatible 😢
waldbaer commented 5 years ago

Isn't eslint 'only' a style checker? So during runtime eslint has no influence. I am no deep nodejs expert but maybe it is possible to use an older eslint version for CI tests with node 4 and 5?

njh commented 5 years ago

Yes, it is only a build-time dependency, not a run-time dependency. However there is a higher risk of depending on old/unsupported software depending on a package that steals your NPM key/bitcoins/SSH keys/password safe/documents.

In ruby I have successfully changed dependencies based on the version of ruby but I am not sure how to achieve that with node - the configuration is JSON not JavaScript.

The other issue is that the standard.js style guide changes when you upgrade it and it doesn't support older versions of node.js. I think maybe the solution is to take the configuration of an older version of standard.js and then stick to that.

waldbaer commented 5 years ago

Or simply remove the support for node 4 + 5? But I have no experience how many people / projects are still using these versions.

Maybe this blog post gives some input: https://nodered.org/blog/2018/08/14/version-0-19-released

"Node.js 4 reached its end-of-life back in April"

njh commented 5 years ago

I have done as you suggested and removed testing for Nodes version 4 and 5 and updated the README to say that node 6 or higher is required.

Hopefully your PR will pass now...

waldbaer commented 5 years ago

Can you please trigger a new travis CI run? I did not find any button to do this.

njh commented 5 years ago

Hm. I have tried triggering a rebuild a couple of times but it looks like it is still using the old .travis.yml file.

Can you try rebasing your branch please?

waldbaer commented 5 years ago

OK. I rebased it. And now it is passing CI.

njh commented 5 years ago

Awesome, thanks for your persistence in getting this merged.