Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve dag API #4133

Merged
merged 8 commits into from
Aug 16, 2017
Merged

Improve dag API #4133

merged 8 commits into from
Aug 16, 2017

Conversation

magik6k
Copy link
Member

@magik6k magik6k commented Aug 9, 2017 •
edited
Loading

Closes #3771

  • Added --hash to put
  • --input-enc cbor input 'encoder'
  • dag-pb support in put

TODO:

  • Output cbor encoding
  • Have the js-ipfs-api PR working

Depends on ipfs/go-ipld-cbor#24

cc @diasdavid

@magik6k magik6k force-pushed the feat/improve-dag-api branch from cced242 to 84a570d Compare August 9, 2017 19:46
daviddias
daviddias reviewed Aug 11, 2017
View reviewed changes
core/commands/dag/dag.go
@@ -48,6 +50,7 @@ into an object of the specified format.
cmds.StringOption("format", "f", "Format that the object will be added as.").Default("cbor"),
cmds.StringOption("input-enc", "Format that the input object will be.").Default("json"),
cmds.BoolOption("pin", "Pin this object when adding.").Default(false),
cmds.StringOption("hash", "Hash function to use").Default(""),
Copy link
Member

Choose a reason for hiding this comment

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

can this be hash-alg to match https://github.com/ipfs/interface-ipfs-core/tree/master/API/dag#dag-api ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's hash in ipfs add: https://github.com/ipfs/go-ipfs/blob/master/core/commands/add.go#L44, I guess it's better for consistency

Copy link
Member

Choose a reason for hiding this comment

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

Fair point.

@daviddias
Copy link
Member

@magik6k woot! Thanks for pushing this forward. Did you get to test it with the WIP PR on js-ipfs-api? Ideally we should land both at the same time so that we are 100% convinced that is what we want.

@magik6k
Copy link
Member Author

magik6k commented Aug 11, 2017

Ran the tests now:

@daviddias
Copy link
Member

daviddias commented Aug 11, 2017 •
edited
Loading

@magik6k you are welcome to :) Thank you!

@magik6k magik6k force-pushed the feat/improve-dag-api branch from 49afaa6 to b89a579 Compare August 11, 2017 23:30
@whyrusleeping
Copy link
Member

@magik6k LGTM, though the OSX sharness tests appear to be failing. Use printf instead of echo, it seems to work better cross platform.

@magik6k
Copy link
Member Author

magik6k commented Aug 12, 2017

Yeah, I'll have a look at it. Don't merge yet as I want to get ipfs-inactive/js-ipfs-http-client#534 fully functional with this

@magik6k magik6k mentioned this pull request Aug 12, 2017
Merged
@magik6k magik6k force-pushed the feat/improve-dag-api branch from 05694f7 to 8014acb Compare August 12, 2017 19:46
magik6k added 6 commits August 12, 2017 21:49
@magik6k
dag: add option to specify hash
0f234d7 
License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>
@magik6k
dag: fix git plugin
7f208ce 
License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>
@magik6k
dag: test --hash option
1fa6113 
License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>
@magik6k
dag: cbor input-enc
6bf019a 
License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>
@magik6k
dag: add dag-pb support to put
0953295 
License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>
@magik6k
dag: sharness tests for dag-pb put
f8b17c6 
License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>
@magik6k magik6k force-pushed the feat/improve-dag-api branch from 8014acb to f8b17c6 Compare August 12, 2017 19:49
@magik6k
dag: dag resolve subcommand
5da8368 
License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>
@magik6k magik6k force-pushed the feat/improve-dag-api branch from 6fe1474 to 5da8368 Compare August 13, 2017 18:58
@magik6k magik6k added this to the Ipfs 0.4.11 milestone Aug 13, 2017
@magik6k
dag: fix codeclimate
30f482e 
License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>
@magik6k magik6k force-pushed the feat/improve-dag-api branch from 11a0ba9 to 30f482e Compare August 16, 2017 20:29
@whyrusleeping whyrusleeping merged commit 94b832d into master Aug 16, 2017
@whyrusleeping whyrusleeping deleted the feat/improve-dag-api branch August 16, 2017 22:22
@daviddias
Copy link
Member

@whyrusleeping I believe this wasn't done yet. See @magik6k comment

Yeah, I'll have a look at it. Don't merge yet as I want to get ipfs-inactive/js-ipfs-http-client#534 fully functional with this

@magik6k
Copy link
Member Author

magik6k commented Aug 17, 2017

Yes, it was merged as I wanted to put the larger changes into separate PR.

@ghost ghost mentioned this pull request Aug 21, 2017
Merged
@magik6k magik6k restored the feat/improve-dag-api branch November 27, 2017 03:34
@magik6k magik6k deleted the feat/improve-dag-api branch November 27, 2017 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daviddias daviddias daviddias left review comments

@whyrusleeping whyrusleeping whyrusleeping approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Ipfs 0.4.11
Development

Successfully merging this pull request may close these issues.
3 participants
@magik6k @daviddias @whyrusleeping