-
Notifications
You must be signed in to change notification settings - Fork 299
interface-ipfs-core config API spec compliant #307
Conversation
daviddias
commented
Jun 30, 2016
} | ||
|
||
if (!key) { | ||
return send('config/show', null, null, null, true, callback) |
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.
Probably be valuable for the send
method to accept a config argument rather than having a parameter list filled with null
s
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.
💯 agree. In fact, what I'm very much thinking is after merging the config-api PR into ipfs-api/ng branch, just make a PR to update how request-api.js works, break the tests (so that we don't have to run 300 tests while developing a single feature) and clean up whatever else I find.
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.
It is happening :) 43a1dfa
a318463
to
be4f964
Compare
if (typeof config === 'object') { | ||
config = streamifier.createReadStream(new Buffer(JSON.stringify(config))) | ||
return send('config/replace', null, null, config, callback) | ||
} |
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.
Just missing to make the config.replace tests to work. It seems that this was never working (see https://github.com/ipfs/js-ipfs-api/blob/master/test/api/config.spec.js#L123-L133), unfortunately.
The current problem is that the config never gets replaced and after we send the replace, the content type of the following config.get requests becomes text/plain instead of application/json
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.
Learned that this is an error in actual go-ipfs ipfs/kubo#2927