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

refs endpoint #978

Merged
merged 8 commits into from
May 11, 2019
Merged

refs endpoint #978

merged 8 commits into from
May 11, 2019

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Apr 24, 2019

@ghost ghost assigned dirkmc Apr 24, 2019
@ghost ghost added the in progress label Apr 24, 2019
@dirkmc dirkmc mentioned this pull request Apr 24, 2019
2 tasks
@dirkmc dirkmc requested a review from alanshaw April 24, 2019 06:28
Copy link
Contributor

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Drive-by comment: refsReadableStream is missing (ls and others have ReadableStream versions)
Missed it or was it left out on purpose?

@dirkmc
Copy link
Contributor Author

dirkmc commented Apr 27, 2019

@lidel I didn't see it used anywhere so I didn't add it. Do you know if it will be needed?

FYI I am refactoring this PR heavily to

  • move most of the tests into interface-ipfs-core, so they can be used to test against go and js
  • traverse the refs DAG in js-ipfs instead of relying on js-ipfs-unix-exporter, which is specifically aimed at unix format files (turns out this code is quite simple to implement with pull-traverse)

@dirkmc
Copy link
Contributor Author

dirkmc commented Apr 28, 2019

I added refsReadableStream in case it will be needed

@lidel
Copy link
Contributor

lidel commented Apr 29, 2019

Thanks! FYSA we always provide both pull-stream and readable-stream versions (see interface-js-ipfs-core/SPEC/FILES), and if streaming is supported users expect to find both.

Copy link
Contributor

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

Please update the README to include links to the docs in interface-ipfs-core.

src/files-regular/refs-local-pull-stream.js Show resolved Hide resolved
src/files-regular/refs.js Outdated Show resolved Hide resolved
src/files-regular/refs-local-pull-stream.js Outdated Show resolved Hide resolved
@dirkmc dirkmc requested a review from alanshaw May 3, 2019 15:54
@alanshaw
Copy link
Contributor

@dirkmc I think this just needs a rebase to get the tests passing - go-ipfs 0.4.20 is broken for adding files so we've reverted to testing against 0.4.19.

@dirkmc
Copy link
Contributor Author

dirkmc commented May 10, 2019

@alanshaw thanks, rebased

@alanshaw alanshaw merged commit a741e10 into master May 11, 2019
@alanshaw alanshaw deleted the feat/refs branch May 11, 2019 11:49
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.

4 participants