-
-
Notifications
You must be signed in to change notification settings - Fork 177
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
Location was removed from abstract-leveldown #494
Conversation
ralphtheninja
commented
Jul 11, 2018
- handle location in LevelDOWN constructor
- pass a factory function to abstract tests
- use tempy and remove cleanup code
* handle location in LevelDOWN constructor * pass a factory function to abstract tests * use tempy and remove cleanup code
@vweevers How do you see us using this branch? Will it be a long lasting branch? It will always target |
Or rebase to remove the first commit, then next can be merged into master. I don't know, we'll see, also depends on what other stuff happens on master in the mean time. Main reason I suggested this approach is to have a way to canary test |
It's a good idea. We should have this on |
leveldown.js
Outdated
AbstractLevelDOWN.call(this, location) | ||
AbstractLevelDOWN.call(this) | ||
|
||
this.location = location |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'non-string location leveldown() throws' | ||
) | ||
t.end() | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This build will fail since abstract-leveldown#master has moved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should target commit refs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funny thing, the tests doesn't fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should target commit refs?
I'll target a commit ref in next PR.
* Location was removed from abstract-leveldown * handle location in LevelDOWN constructor * pass a factory function to abstract tests * use tempy and remove cleanup code * Add type check for location in constructor
* Location was removed from abstract-leveldown * handle location in LevelDOWN constructor * pass a factory function to abstract tests * use tempy and remove cleanup code * Add type check for location in constructor