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

Include changeset id in deletion #35

Closed
wants to merge 1 commit into from
Closed

Conversation

gmaclennan
Copy link
Member

@gmaclennan gmaclennan commented Feb 4, 2017

@noffle could you review and add a test here (see the failing test in osm-p2p-server https://github.com/digidem/osm-p2p-db/compare/changeset-deletions) attempts to fix #34

@gmaclennan gmaclennan requested a review from hackergrrl February 4, 2017 02:20
@hackergrrl
Copy link
Contributor

What do you think about just having fields.v set to opts.value directly? This makes it more versatile for the future.

@gmaclennan
Copy link
Member Author

That makes sense to me. I'm not sure about @substack's original motivation for choosing this API.

@hackergrrl
Copy link
Contributor

Can you also update the osm.del() calls we make to set opts.value.changeset?

@@ -172,6 +172,9 @@ DB.prototype._del = function (key, opts, cb) {
fields.refs.push.apply(fields.refs, v.refs)
}
})
if (opts.value && opts.value.changeset) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just checking for opts.changeset?

@hackergrrl
Copy link
Contributor

Let's nix this PR and update osm-p2p-db#del() to accept a value on deletions, just like PUTs, like we discussed in meatspace.

@gmaclennan
Copy link
Member Author

closing in favour of #46

@gmaclennan gmaclennan closed this Jun 2, 2017
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

Successfully merging this pull request may close these issues.

Changeset is not stored on deleted elements
2 participants