Skip to content
This repository was archived by the owner on Mar 10, 2020. It is now read-only.

docs: change config.get() to object #343

Closed
wants to merge 1 commit into from
Closed

docs: change config.get() to object #343

wants to merge 1 commit into from

Conversation

hacdias
Copy link
Contributor

@hacdias hacdias commented Aug 3, 2018

@ghost ghost assigned hacdias Aug 3, 2018
@ghost ghost added the in progress label Aug 3, 2018
@hacdias hacdias changed the title [WIP] docs: change config.get() to object docs: change config.get() to object Aug 3, 2018
@hacdias hacdias requested review from alanshaw and daviddias August 3, 2018 11:14
@hacdias
Copy link
Contributor Author

hacdias commented Aug 4, 2018

@diasdavid @alanshaw what do you think? 😄

@@ -14,7 +14,7 @@

`key` is the key of the value that should be fetched from the config file. If no key is passed, then the whole config should be returned. `key` should be of type String.

`callback` must follow `function (err, config) {}` signature, where `err` is an error if the operation was not successful and `config` is a JSON object containing the configuration of the IPFS node.
`callback` must follow `function (err, config) {}` signature, where `err` is an error if the operation was not successful and `config` is an object containing the configuration of the IPFS node.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you saying that the config is not pure JSON?

Copy link
Contributor Author

@hacdias hacdias Aug 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config is a JavaScript object, which can be stringified to JSON. I believe a js object is more handy.

Please see ipfs-inactive/js-ipfs-http-client#822 and ipfs-inactive/js-ipfs-http-client#825 😄

Right now, the API returns a Buffer for go-ipfs and an object for js-ipfs.

The js-ipfs core seems to return an object too and not a Buffer.

Hence my suggestion to return an object everywhere. Otherwise the API will behave differently with js-ipfs and go-ipfs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be an object everywhere. What we mean by it being a JSON object is that it should include no buffers on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diasdavid Oh, makes sense then. I thought "JSON object" == "Buffer with JSON string". So this PR can be closed, but ipfs-inactive/js-ipfs-http-client#825 should be merged then.

@hacdias hacdias closed this Aug 5, 2018
@ghost ghost removed the in progress label Aug 5, 2018
@hacdias hacdias deleted the fix-config branch August 5, 2018 07:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants