Skip to content
This repository was archived by the owner on Dec 2, 2024. It is now read-only.

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

Closed
vweevers wants to merge 8 commits intomasterfrom
refactor-open
Closed

Refactor open() and close() to mostly bypass levelup#79
vweevers wants to merge 8 commits intomasterfrom
refactor-open

Conversation

@vweevers
Copy link
Copy Markdown
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
Comment thread leveldown.js Outdated
@vweevers
Copy link
Copy Markdown
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
Copy Markdown
Member

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

😆

@vweevers vweevers force-pushed the refactor-open branch 2 times, most recently from cbd6b9a to 5119ac0 Compare October 3, 2019 18:17
@vweevers vweevers mentioned this pull request Oct 8, 2019
@vweevers

This comment has been minimized.

@vweevers
Copy link
Copy Markdown
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
Copy Markdown
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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

refactor Requires or pertains to refactoring semver-major Changes that break backward compatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Applies prefix twice on nested sublevel An in-range update of abstract-leveldown is breaking the build 🚨 Segfault with subleveldown

2 participants