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

README and API improvements #8

Merged
merged 5 commits into from Mar 22, 2016
Merged

Conversation

hackergrrl
Copy link
Contributor

Adds a fancy-pants README, and also makes path and dagService non-optional parameters.

Addresses #9

@@ -1,5 +1,4 @@
5
" $��G�,�A�4{���x�Z/.����D`� 200Bytes.txt�3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was already not passing as of master.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was failing because git doesn't check in empty dirs. @nginnever has pushes a fix to his branch https://github.com/ipfs/js-ipfs-data-importing/pull/6/files which essentially creates the empty folders if they are not there..

Copy link
Contributor

Choose a reason for hiding this comment

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

@noffle could you redo this PR with just the README improvs. @nginnever PR will be merged soon which adds the expected behaviour on the test. Sorry that you had to go through that trouble.

@daviddias
Copy link
Contributor

@noffle #6 was merged, you should see the nested dir test passing on master now. Could you redo your PR with the readme changes? Thank you :)

@hackergrrl
Copy link
Contributor Author

Changes made. I'm also multiplexing on the target param instead of having options.{buffer|path|stream} and the complexity that introduces, since we only ever want one of these.

@hackergrrl
Copy link
Contributor Author

Blocked on #10 for now.

@daviddias
Copy link
Contributor

greeeen #12 :)

@hackergrrl
Copy link
Contributor Author

The js-ipfs-merkle-dag changes don't make a difference for me -- Chrome and Firefox both still crash /wo logs.

@hackergrrl
Copy link
Contributor Author

Tests are passing!

The home-made chunker used was allocating ~O(100n) bytes, causing the 1.2mb file to crash Chrome and Firefox. I switched us to block-stream2, which is O(n) and supports zero-padding too.

@hackergrrl hackergrrl mentioned this pull request Mar 22, 2016
@daviddias
Copy link
Contributor

woooo :)

daviddias added a commit that referenced this pull request Mar 22, 2016
README and API improvements
@daviddias daviddias merged commit 782c862 into ipfs-inactive:master Mar 22, 2016
@nginnever
Copy link
Contributor

Removing chunker test file removed the node environment testing of the fixed size chunker, the test now only runs against the browser.

Is this desired?

The test wasn't set up to completely reduce redundancy and the same tests were in two different places and required separately instead of two requires to the same test file like in merkle-dag

@daviddias
Copy link
Contributor

@nginnever missed that. We don't need to test the chunker anymore cause we are using block-stream2 which does it well for us. @noffle mind cleaning that part as well? Thank you both :)

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