add node.deleted to indicate if a node is deleted #122
Conversation
|
/cc @andrewosh |
lib/iterator.js
Outdated
| @@ -204,7 +204,8 @@ function byKey (a, b, reverse) { | |||
|
|
|||
| function allDeletes (nodes) { | |||
| for (var i = 0; i < nodes.length; i++) { | |||
| if (nodes[i].value !== null) return false | |||
| if (nodes[i].value === null || nodes[i].deleted) continue | |||
There was a problem hiding this comment.
So we're saying that null is still going to represent a deletion (as in, even with this addition one wouldn't be able to have meaningful null values)?
There was a problem hiding this comment.
Definitely necessary for backwards-compat, but is that a desirable limitation once this change is ancient history?
There was a problem hiding this comment.
No I'd like to get rid of that of check later on. Should we just drop support for null === deleted in 3.0.0 stable? I'm okay doing that
There was a problem hiding this comment.
Yeah especially since 3.0.0 is considered an "LTS" release right?
|
@andrewosh updated. wdyt? |
| @@ -44,6 +44,9 @@ PutRequest.prototype._finalize = function () { | |||
| clock: this._clock | |||
| } | |||
|
|
|||
| // TODO: allow to set this flag explicitly | |||
| if (node.value === null) node.deleted = true | |||
There was a problem hiding this comment.
Still want this one in there?
There was a problem hiding this comment.
Ah yea for now I need to keep it as that's how deletes are implemented internally (simply as db.put(key, null)). We can rewrite that at some point though
There was a problem hiding this comment.
K. So this PR will just cover the get side of things, and null puts will still be invalid for the time being?
There was a problem hiding this comment.
Yea they'll turn into a delete (but we'll fix that asap)
mafintosh
force-pushed
the
add-node-deleted-flag
branch
from
862adf6
to
2f7173c
Compare
Backwards compat. Should make it possible to delete with a value which is useful for resolving conflicts