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

draft api docs for blocks #15

Merged
merged 4 commits into from
May 17, 2016
Merged

Conversation

hackergrrl
Copy link
Contributor

@hackergrrl hackergrrl commented May 17, 2016

  1. del was intentionally omitted. It's not in js-ipfs-api, so let's not commit to it until there's a clear need / demand.
  2. js-ipfs-api and js-ipfs core disagree on the return object of block.stat. The former is way more descriptive than the latter, so we can go with that.

@daviddias
Copy link
Contributor

js-ipfs-api and js-ipfs core disagree on the return object of block.stat. The former is way more descriptive than the latter, so we can go with that.

Can we provide the same level of information in js-ipfs-api?

@hackergrrl
Copy link
Contributor Author

Good catch! I looked and no -- no we can not. go-ipfs doesn't provide more than Key and Size, so we'll need to just provide those.

@daviddias
Copy link
Contributor

we can probably shim that by making one more call?

@hackergrrl
Copy link
Contributor Author

LGTM to merge this?

@hackergrrl hackergrrl merged commit e9c15c2 into ipfs-inactive:master May 17, 2016
@daviddias
Copy link
Contributor

Oh, let's not merge without tests and the tests being fully applied to both js-ipfs and js-ipfs-api. We don't want to expose a interface that isn't being used yet. We want to make sure that this document can be used as reference for implementation and for usage.

@daviddias
Copy link
Contributor

Also, another thing is that we don't want to nail down a spec for API without getting a feeling for it during implementation process.

//cc @dignifiedquire @nginnever

@dignifiedquire
Copy link
Contributor

Done in #16

@daviddias
Copy link
Contributor

Thank you @dignifiedquire. @noffle I believe you have open a PR again

@hackergrrl
Copy link
Contributor Author

hackergrrl commented May 17, 2016

I'm in favour of getting docs merged once they are LGTM'd to make it clear they're accepted, and then work on the impl in a separate PR. We can make intentions clear at the top of the readme that this is very WIP / pre-release until then. Leaving lots of things open makes it less clear what we're LGTM on and what is still in progress.

@daviddias
Copy link
Contributor

Somethings result from the experience of actually making the tests and adapting the API. Writing a spec and not testing it will lead to trouble

@hackergrrl
Copy link
Contributor Author

Accepting a PR into master is not analogous to it being set in stone -- especially during early development. It gives us an agreed upon vector, with the knowledge that it can change as needed.

@hackergrrl
Copy link
Contributor Author

Not worth further bikeshedding on -- let's just get the impl in. 😄

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.

3 participants