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

Refactor open() and close() to mostly bypass levelup #79

Closed
wants to merge 8 commits into from

Conversation

vweevers
Copy link
Member

@vweevers vweevers commented Sep 30, 2019

Closes #60, closes #77, closes #78, and opens up the possibility to add a manifest.

I was sitting around thinking "how can i make subleveldown more complicated" and this PR is the result.

@vweevers vweevers added refactor Requires or pertains to refactoring semver-major Changes that break backward compatibility labels Sep 30, 2019
leveldown.js Outdated Show resolved Hide resolved
@vweevers
Copy link
Member Author

To simplify, maybe we should say that a user must manage the open state of a db outside of subleveldown. Meaning:

  • You must pass in either an opening or open db, else we throw
  • Closing a sublevel is a noop. To me it feels wrong for a sublevel to close its "parent" db.
  • It should however be valid to reopen a closed sublevel - but it should not touch anything except subleveldown state.

@vweevers

This comment has been minimized.

@ralphtheninja
Copy link
Member

I was sitting around thinking "how can i make subleveldown more complicated" and this PR is the result.

😆

@vweevers

This comment has been minimized.

@vweevers
Copy link
Member Author

Alternative strategy:

  • Only accept a levelup db as input (breaking change), so that we can hook into open events
  • Check that by if (db.supports.deferredOpen) so we can support abstract-leveldown db's later on
  • Make that if (db.supports.deferredOpen || reachdown.is(db, 'levelup') for backward compat
  • Never open or close the db. Instead merely wait for it.

@vweevers
Copy link
Member Author

In other words, the input db must open itself (or once closed by the user, be explicitly reopened by the user) because sublevels shouldn't concern themselves with (the state of) the rest of the db.

Closing this PR, I don't like it. Will explore the alternative strategy.

@vweevers vweevers closed this Oct 11, 2019
@vweevers vweevers deleted the refactor-open branch October 11, 2019 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Requires or pertains to refactoring semver-major Changes that break backward compatibility
Projects
None yet
2 participants