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

Support options passed to .open #660

Merged
merged 1 commit into from
Jun 28, 2019

Conversation

achingbrain
Copy link
Contributor

LevelDown supports an optional options argument passed the db.open method. Currently LevelUp fails if options are passed to .open as it tries to invoke them as the callback so this PR adds support for that.

By and large you don't call .open directly but if you do (or an intermediate module does as in my case) LevelUp should probably honour the api.

LevelDown supports an [optional `options` argument](https://github.com/Level/abstract-leveldown#dbopenoptions-callback)
passed the `db.open` method.  Currently LevelUp fails if options are
passed to `.open` so this PR adds support for that.
@vweevers
Copy link
Member

vweevers commented Jun 25, 2019

Options that you pass to the levelup constructor are passed on to db.open().

And personally I'm hoping we can move away from db.open() options altogether. It's much easier to "compose" (combine) modules if they take their options in the constructor, like encoding-down, level-js and a few others do.

@achingbrain
Copy link
Contributor Author

Options that you pass to the levelup constructor are passed on to db.open()

That behaviour is maintained.

I'm hoping we can move away from db.open() options altogether

Sounds like a good plan, but if the goal here is to support the same API as abstract-leveldown but with added streams, then support for passing options to db.open should be in there until it's removed from abstract-leveldown itself.

@vweevers
Copy link
Member

but if the goal here is to support the same API as abstract-leveldown but with added streams

That's not - or has not been - the goal. However there is a long-term goal to merge levelup functionality into abstract-leveldown. So on second thought, it's good to bring the two interfaces closer together :) I'm in

@vweevers vweevers added enhancement New feature or request semver-minor New features that are backward compatible labels Jun 26, 2019
@vweevers vweevers self-requested a review June 26, 2019 10:14
@vweevers vweevers merged commit 9071230 into Level:master Jun 28, 2019
@vweevers
Copy link
Member

This needs a follow-up; browser tests are failing on the use of arrow functions.

vweevers added a commit that referenced this pull request Jun 28, 2019
@vweevers
Copy link
Member

4.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request semver-minor New features that are backward compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants