Skip to content
This repository was archived by the owner on Dec 1, 2024. It is now read-only.
/ levelup Public archive

Levelup from leveldown#342

Closed
ralphtheninja wants to merge 10 commits intoLevel:masterfrom
ralphtheninja:levelup-from-leveldown
Closed

Levelup from leveldown#342
ralphtheninja wants to merge 10 commits intoLevel:masterfrom
ralphtheninja:levelup-from-leveldown

Conversation

@ralphtheninja
Copy link
Copy Markdown
Member

Re-creating #299 as a new PR with cherry picked commits. Also using isLevelDOWN() from abstract-leveldown.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why is that still a function? i think now is a good time to enable options.db = down

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ah I see in the tests that that already works, so we can simplify here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes. This if-statement looks terrible :) Imo the best would be to remove the location parameter completely.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shall me make it only this:

var db = levelup(leveldown(location))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Skip the callback completely?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm all for simplifying, but I'd prefer to do all of that in a separate PR. I guess that change will involve a new major?

@juliangruber
Copy link
Copy Markdown
Member

given #299 (comment), we should document that the leveldown must not have been opened

@ralphtheninja
Copy link
Copy Markdown
Member Author

Agree. I'll update README.

@Level Level locked and limited conversation to collaborators May 18, 2015
@ralphtheninja
Copy link
Copy Markdown
Member Author

Locking PR until leveldown can keep track of its own open state.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants