Level / memdown

In-memory abstract-leveldown store for Node.js and browsers.
MIT License
287 stars 37 forks source link

(#13) - use binary search for options.start #14

Closed nolanlawson closed 10 years ago

nolanlawson commented 10 years ago

FWIW, this also passes the PouchDB test suite.

The only thing I'm concerned about is the exclusiveStart option. It seems like it should be XORed with key === options.start rather than just ORed, but the test suites pass anyway. Also confused as to why the localstorage-down tests pass even though there's no mention of exclusiveStart in that code whatsoever.

nolanlawson commented 10 years ago

Ah okay, it looks like localstorage-down still depends on an older version of abstract-leveldown.

rvagg commented 10 years ago

I think I'm OK with this if it's passing the tests but I'd like @mhart to have a look at it first.

mhart commented 10 years ago

All looks good to me! @rvagg you happy with the style (semicolons, etc)?

nolanlawson commented 10 years ago

I didn't notice the consistent omission of semicolons. I'll fix that and re-push.

mhart commented 10 years ago

Excellent - just tested this with dynalite and it works a treat (and passes all tests there) - happy to merge if you like @rvagg

rvagg commented 10 years ago

:thumbsup:

mhart commented 10 years ago

:shipit:

mhart commented 10 years ago

Thanks @nolanlawson! Certainly should be quicker when using options.start with large key spaces.

rvagg commented 10 years ago

done, as @0.9.0 thanks @nolanlawson, I'll add you as a collaborator to the project too, i hope you don't mind!

nolanlawson commented 10 years ago

Sounds good, and thanks for the quick merge!