fbzhong / node-cityhash

NodeJS bindings for Google CityHash
https://github.com/fbzhong/node-cityhash
MIT License
28 stars 4 forks source link

Update to CityHash v1.1 #7

Open rmg opened 11 years ago

rmg commented 11 years ago

I've got a branch that updates CityHash to v1.1, but all of the tests fail. I think it is because the hash functions have actually changed and the tests are still valid, but I'm not 100% certain if that's expected or not.

If it is, I'll just update the tests and submit a pull request.

yyfrankyy commented 11 years ago

Yes, there is some compatibility issue, but I am sure that old version is still needed.

If you can send a PR with compatibility in both version, I think @fbzhong will happy to merge.

rmg commented 11 years ago

@yyfrankyy do you mean include both versions of the CityHash C++ library in the module?

yyfrankyy commented 11 years ago

Yes.

rmg commented 11 years ago

Hmm.. interesting idea.

Here's a few options that come to mind

var cityhash = require('cityhash')
  , hash64 = cityhash.hash64           // default to 1.0.3
  , cityhash11x = cityhash.v11x
  , new_hash64 = cityhash.v11x.hash64   // allow access to 1.1.0

Or maybe something like:

var cityhash10x = require('cityhash')         // default to 1.0.3
  , cityhash11x = require('cityhash/1.1.0')   // allow access to 1.1.0

I'm not a fan of using config strings, but it's also an option:

var cityhash = require('cityhash')
cityhash === cityhash.engine('1.0.3')      // top level == existing API
cityhash.engine('1.1.0').hash64("something to hash with 1.1.0")
cityhash.engine('1.0.3').hash64("something to hash with 1.0.3")
cityhash.hash64("uses 1.0.3 at top level to maintain compatibility")

Any preferences for the API?

yyfrankyy commented 11 years ago

Option 2 and 3 looks good to me. or we can just release a new version of citihash (1.x) to npm, and stop adding new feature to 0.x version (just bugfix).

rmg commented 11 years ago

Oh, right.. I always forget about maintaining multiple versions! I'll keep an eye out fro @fbzhong to make a branch and submit a pull request when I see it.