-
Notifications
You must be signed in to change notification settings - Fork 8
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
Test and fix post compat #426
Conversation
@staltz review if you have time :) otherwise i could pass to mix probably |
damn that's not something i expected to fail
|
so uugh code seems to be supposed to allow importing db1 and db2 at the same time? which breaks with this. trying out removing post compat from the default imports |
yay works now |
i guess you can argue this is a major version since it changes the default imports but eeh no one uses those anyway |
@staltz done for real now |
Benchmark results
|
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.
Looks good to me
@@ -18,7 +18,7 @@ mkdirp.sync(dir) | |||
|
|||
const keys = ssbKeys.loadOrCreateSync(path.join(dir, 'secret')) | |||
|
|||
const sbot = SecretStack({ appKey: caps.shs }).use(require('../')).call(null, { | |||
const sbot = SecretStack({ appKey: caps.shs }).use(require('../')).use(require('../compat/post')).call(null, { |
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.
why is this one in here?
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.
because db.post.value
is used further down in these tests
@@ -8,6 +8,5 @@ module.exports = [ | |||
require('ssb-box'), | |||
require('ssb-box2'), | |||
require('./compat/publish'), | |||
require('./compat/post'), |
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 is a breaking change, so be careful to make a new major version.
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.
Seems ok
v8.0.0 |
Benchmark results
|
For ssbc/ssb-tribes#105
Trying to remove db1 completely from ssb-tribes, apparently some test runs replicate and that depends on
sbot.post
. But the compat plugin was only providingsbot.db.post
. I also added a test.