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

Normalize KEY API #192

Merged
merged 9 commits into from
Jan 5, 2018
Merged

Normalize KEY API #192

merged 9 commits into from
Jan 5, 2018

Conversation

hacdias
Copy link
Contributor

@hacdias hacdias commented Jan 3, 2018

Ref.: ipfs-inactive/js-ipfs-http-client#659

Sorry, something went wrong.

@ghost ghost assigned hacdias Jan 3, 2018
@ghost ghost added the in progress label Jan 3, 2018
@hacdias hacdias changed the title [WIP] Update KEY.md [WIP] Normalize KEY API Jan 3, 2018
@hacdias hacdias changed the title [WIP] Normalize KEY API Normalize KEY API Jan 3, 2018
@hacdias hacdias requested a review from daviddias January 3, 2018 14:13
@hacdias
Copy link
Contributor Author

hacdias commented Jan 3, 2018

@diasdavid done. Should be merged alongside with ipfs-inactive/js-ipfs-http-client#659.

@richardschneider
Copy link
Contributor

@hacdias PR ipfs/js-ipfs#1133 has the implementation (core, cli and http) for the Key API. Waiting for approval from @diasdavid

With the case change, you will also need to change core to use lower case, because core and http-api must be compatible.

However, http must still return Capitalized names because that is the spec defined by go-ipfs. This allows a go client to use a js-ipfs daemon.

The cli can either talk directly to core or use http-api, so it must be changed to use lower case.

@hacdias
Copy link
Contributor Author

hacdias commented Jan 3, 2018

@richardschneider oops! Didn't notice that PR.

I just made this (and ipfs-inactive/js-ipfs-http-client#659) because most of the JS API is using non-capitalized object fields and that's just like the Standard specification tells us.

As far as I understood:

  • HTTP API must return capitalized names (so js-ipfs and go-ipfs must return them that way).
  • External APIs (go-ipfs-api, js-ipfs-api, py-ipfs-api, ...) should be compliant to each language standard, which, in JS case, is to be non-capitalized.

Did I miss something?

Edit: please check #190 too.

@richardschneider
Copy link
Contributor

@hacdias I think we are on the same page. When these normalize PRs are released, please ping me and I'll update ipfs/js-ipfs#1133

@hacdias
Copy link
Contributor Author

hacdias commented Jan 3, 2018

@diasdavid do you agree with what I said?

@daviddias
Copy link
Contributor

thank you both for figuring this out and reaching consensus. LGTM :)

@daviddias daviddias merged commit 5a21d6c into master Jan 5, 2018
@daviddias daviddias deleted the key-api-normalize branch January 5, 2018 18:01
@ghost ghost removed the in progress label Jan 5, 2018
@hacdias
Copy link
Contributor Author

hacdias commented Jan 5, 2018

Thanks @diasdavid!
Pinging @richardschneider 😄

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.

None yet

3 participants