Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document lt/lte/gt/gte #201

Closed
rvagg opened this issue Oct 1, 2013 · 13 comments
Closed

Document lt/lte/gt/gte #201

rvagg opened this issue Oct 1, 2013 · 13 comments

Comments

@rvagg
Copy link
Member

rvagg commented Oct 1, 2013

LevelDOWN has had this for a little while now but it's not documented. We should remove reference to 'start' and 'end' (perhaps just note that they used to be the way to do it and they are kept for backward-compat).

Docs need both here and in LevelDOWN.

@juliangruber
Copy link
Member

I'd completely remove start and end from docs, the sooner people migrate the better.

@mcollina
Copy link
Member

mcollina commented Oct 1, 2013

oh shit 😟. Are lt/lte/gt/gte supported in other leveldown implementations?

@rvagg
Copy link
Member Author

rvagg commented Oct 1, 2013

@juliangruber I feel the same way but the problem is that there's so much documentation out there now that refer to start and end, I'm about to contribute even more tomorrow at lxjs with a bunch of start & end in my code examples. Best to explain why things are different and that you shouldn't rely on start and end cause we're liable to remove them without notice.

@mcollina good question, I think we have lt/lte/gt/gte in the AbstractLevelDOWN test suite, if not, it should be!

@heapwolf
Copy link
Contributor

heapwolf commented Oct 1, 2013

what about a deprecation notice?

@mcollina
Copy link
Member

mcollina commented Oct 1, 2013

@rvagg it seems not, the tests still refers to start and end: https://github.com/rvagg/node-abstract-leveldown/blob/master/abstract/iterator-test.js#L376-L392

I think we should support all backends before deprecating them here.

@rvagg
Copy link
Member Author

rvagg commented Oct 1, 2013

start & end should still be in all the tests and the new ones should just be additions, until we remove start/end (IF we remove them) they need to be in the tests so they don't rot.

@rvagg
Copy link
Member Author

rvagg commented Oct 1, 2013

@mcollina: https://github.com/rvagg/node-abstract-leveldown/blob/master/abstract/ranges-test.js

If a backend isn't pulling in this test then you should go and put a pull request to that backend to do it.

But yeah, you're probably right, until we know that we have coverage from the 3 leveldb's, memdown, lmdb and level.js we should probably tread carefully.

@mcollina
Copy link
Member

mcollina commented Oct 1, 2013

Oh that was new :).

For sure level.js is not supporting it: https://github.com/maxogden/level.js/blob/master/test.js.

@dominictarr
Copy link
Contributor

Hi, yes there are tests in abstract leveldown - abstract/ranges-test.js
But it needs to be added to level.js and memdown (in particular)
The tests still cover start, end I think we should keep that in for now,
it's a lot of work to rewrite old code, but new code should use lt, gt, lte, gte

@dominictarr
Copy link
Contributor

update - this is now supported in the important leveldowns - Level/level-js#32
it's easy to add support for lte, gte, lt, gt using the ltgt

@dominictarr
Copy link
Contributor

oh... this actually just needs a pull request. doing it.

@TimothyGu
Copy link

This should be closed as #258 was merged.

@ralphtheninja
Copy link
Member

@TimothyGu Correct. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants