maritz / nohm

node.js object relations mapper (orm) for redis
http://maritz.github.io/nohm/
MIT License
500 stars 64 forks source link

Support for ioredis #137

Closed thepipster closed 6 years ago

thepipster commented 6 years ago

Added support for ioredis, issue https://github.com/maritz/nohm/issues/120, by converting uppercase function names to lowercase and reducing connection detection strictness

maritz commented 6 years ago

Thank you very much for this PR!

A couple of questions/requests, since I never really looked into ioredis in depth:

maritz commented 6 years ago

Also, .DS_Store is a macOS specific file, right? Should probably be added to .gitignore instead of adding it to the repo :-)

thepipster commented 6 years ago

Thanks, sorry for delay - I'll take a look.

thepipster commented 6 years ago

So I think ioredis has the equivalent;

(client.connected || client.status === 'connect')

thepipster commented 6 years ago

I made those changes, but the tests are failing (I think) at deleteNonExistant - might need some help to debug...

maritz commented 6 years ago

Wasn't quite as simple as I had hoped, but now all tests pass with node_redis and ioredis.

See #138 for the changes I added. I will probably close this one and then merge #138, unless you want to do some more work on any of this, then I'd recommend pulling in my 1 commit and then we can continue here.

maritz commented 6 years ago

One thing that could still be added, but is entirely optional and could also go into its own PR is ioredis support in extra/stress.js.

Documentation is also missing for now.

thepipster commented 6 years ago

Awesome, thanks!