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

Abstract leveldown v4 #415

Merged
merged 15 commits into from Jan 21, 2018
Merged

Abstract leveldown v4 #415

merged 15 commits into from Jan 21, 2018

Conversation

ralphtheninja
Copy link
Member

@ralphtheninja ralphtheninja commented Dec 22, 2017

Just a temporary branch and a temporary PR to track build status. This PR should never be merged.

Actually, why not just use this PR for updating to abstract-leveldown@4.0.0. It doesn't matter if this PR contains temporary commits, since we can just squash the commits when we're done.

@ralphtheninja
Copy link
Member Author

Current abstract-leveldown#master should be building fine. Once confirmed, I'll switch to abstract-leveldown#close-dbs.

@ralphtheninja
Copy link
Member Author

Switching to abstract-leveldown#close-dbs should fail on Windows.

@ralphtheninja
Copy link
Member Author

@vweevers Seems windows builds are passing now :)

@ralphtheninja
Copy link
Member Author

Will keep this PR open for a while and do amend on the latest commit as soon as master changes for abstract-leveldown so we can re-trigger builds on travis and appveyor.

@ralphtheninja
Copy link
Member Author

@vweevers Interesting. Got some weird error on AppVeyor I haven't seen before

https://ci.appveyor.com/project/Level/leveldown/build/258/job/q90jmulx538af4yg

@ralphtheninja
Copy link
Member Author

Travis has been acting really weird the past days/week. This time it failed to download node 5 binaries with nvm. Restarted that build.

@ralphtheninja
Copy link
Member Author

Travis builds for OSX seem to be borked and have been quite some time.

@vweevers
Copy link
Member

@ralphtheninja do you intend to squash everything into one commit? Because I think there's enough here to warrant separate commits.

@ralphtheninja
Copy link
Member Author

@vweevers Yes, will squash.

@ralphtheninja
Copy link
Member Author

@vweevers I could make a new branch and cherry pick the stuff from this branch that are essential.

@vweevers
Copy link
Member

Or rebase to remove the temporary commits, whatever works.

In any case, the end result LGTM.

@ralphtheninja ralphtheninja merged commit 241e97c into master Jan 21, 2018
@ralphtheninja ralphtheninja deleted the abstract-leveldown-v4 branch January 21, 2018 20:11
@ralphtheninja
Copy link
Member Author

Should we release this as a patch?

@vweevers
Copy link
Member

Not sure. However small it may be, Level/abstract-leveldown@a2621ad is a breaking change, inherited by implementations.

@ralphtheninja
Copy link
Member Author

Then it should be a major.

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.

None yet

2 participants