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

Implement destroy(), use global db store #23

Closed
wants to merge 1 commit into from
Closed

Conversation

nolanlawson
Copy link
Contributor

Fixes #22.

There's now a global store that keeps track of all memdown databases, keyed by location. so if you do:

var db1 = new MemDOWN('foo')
var db2 = new MemDOWN('foo')

then both db1 and db2 will contain the same data.

I also added a destroy test here, rather than in abstract-leveldown, because memdown isn't even passing the latest abstract-leveldown tests. If you want to see these tests pass at 100%, we're going to need to peg abstract-leveldown to v0.12.3 due to the snapshot test.

@nolanlawson
Copy link
Contributor Author

yeah yeah, tests are broken. I say we peg abstract-leveldown

@nolanlawson
Copy link
Contributor Author

rebased, mebbe it'll be green now

while (db.keys.length) {
delete db.store[toKey(db.keys.pop())]
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to delete it out of globalStore too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't need to, but might as well. otherwise an empty array/object hangs around.

Also, you forgot to comment on my semicolon :P

@nolanlawson
Copy link
Contributor Author

updated

@nolanlawson
Copy link
Contributor Author

Missed some more semicolons. When in Rome...

@calvinmetcalf
Copy link
Contributor

NO SEMICOLONS WE ARE IN ROME AND THEY SHOOT SEMICOLONS IN ROME

function getOrCreateDatabaseFromGlobal(name) {
var key = toKey(name)
, db = globalStore[key]
if (!db) {
Copy link
Member

Choose a reason for hiding this comment

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

whitespace before this if

Copy link
Member

Choose a reason for hiding this comment

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

also it doesn't need braces since it's trivial

@rvagg
Copy link
Member

rvagg commented Oct 2, 2014

fine by me with minor style tweaks

@nolanlawson
Copy link
Contributor Author

sorry for the delay. de-Crockfordized the code and ready to push now

@nolanlawson
Copy link
Contributor Author

fe7c13e

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.

Implement destroy()
3 participants