[WIP] Store an optional value on doc deletions.#46
[WIP] Store an optional value on doc deletions.#46hackergrrl wants to merge 18 commits intomasterfrom
Conversation
6f69d95 to
b2ccd9d
Compare
gmaclennan
left a comment
There was a problem hiding this comment.
I feel pretty good about this approach. Any blockers on moving ahead with this?
index.js
Outdated
| var v = row.value.v, d = row.value.d | ||
| if (v && v.lat !== undefined && v.lon !== undefined) { | ||
| var k = row.value.k | ||
| if (k && v.lat !== undefined && v.lon !== undefined) { |
There was a problem hiding this comment.
I don't quite understand this change, can you explain the implications / reasons?
There was a problem hiding this comment.
Now that DELETEs can have a v (value) as well as PUTs, checking for k (key) is a more reliable way of differentiating between PUTs and DELETEs. I can wrap this in a helper function to be more explicit -- not a bad idea.
There was a problem hiding this comment.
Given this is is performance-critical code I think better use a comment rather than function, which would have a slight performance overhead.
One thing though, a delete could have lat and lon defined, that wouldn't affect here right?
There was a problem hiding this comment.
Good call re comment vs function.
One thing though, a delete could have lat and lon defined, that wouldn't affect here right?
Right. Deletions have a d key, and insertions have a k key. Since this new code checks for the presence of a k, even deletions with values would be ignored.
b2ccd9d to
abfa82d
Compare
|
This is ready for another look-over. Here's the gist:
|
eff96fa to
25c5234
Compare
No description provided.