-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Conversation
Currently blocked by tests on master failing all together |
d874314
to
0567881
Compare
0567881
to
5259e7d
Compare
Started seeing these now:
Investigating, might be related with recent js-ipfsd-ctl release Nope. it is really something I'm doing here. |
src/core/components/files-mfs.js
Outdated
@@ -3,38 +3,37 @@ | |||
const promisify = require('promisify-es6') | |||
const mfs = require('ipfs-mfs/core') | |||
|
|||
module.exports = self => { | |||
const mfsSelf = Object.assign({}, self) |
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.
@achingbrain I'm not entirely certain of all the consequences of doing this (given all the internal state that is kept by so many pieces. It doesn't look like a good practice or something that we want to debug later. If you know this is super safe, please do enlighten me as I would love to understand how.
On second note, what we really want is to avoid circular dependencies. js-ipfs-mfs should only take from IPFS what it needs and not the whole object.
src/core/components/files-mfs.js
Outdated
return mfs(mfsSelf, mfsSelf._options) | ||
module.exports = (self) => { | ||
const proxy = { | ||
add: self.add, |
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 the only call I saw MFS using other than dag.get and dag.put
MFS is not happy when it doesn't have the full IPFS object
Will continue to investigate |
Is there anyone available to take this to the finish line? I'm afraid I won't be able to do it for another 1,5/2 weeks |
With ipfs-inactive/interface-js-ipfs-core#378 merged, this PR and respective release becomes P0 - Critical as now the documentation says something different from what js-ipfs does. |
@daviddias - on it! I'll get this to a state where it can be merged. I need to add tests to I'll temporarily skip the |
b53b7c0
to
e0f63bf
Compare
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
For ipfs-inactive/interface-js-ipfs-core#378