Optimization 1 for switchToBasic function in unsharding PR
#101
Conversation
schomatis
force-pushed
the
schomatis/directory/sharding/switchToBasic-optimization-1
branch
from
f633ab3
to
72c69ed
Compare
|
|
||
| // addLinkChild adds the link as an entry to this directory under the given | ||
| // name. Plumbing function for the AddChild API. | ||
| func (d *BasicDirectory) addLinkChild(ctx context.Context, name string, link *ipld.Link) error { |
There was a problem hiding this comment.
nit: the name seems redundant given that ipld.Link already includes a name.
(I mean, ideally we could just use a CID... but we need the size because dagpb).
There was a problem hiding this comment.
My first version was actually just the link argument with the name packed inside but looking at how we manipulate links in go-merkledag it seemed like the idiomatic way to go was to maniuplate that separately. Even internally here in go-unixfs we handle the name separately.
I totally agree with you and the current way seems a bit counter-intuitive to me so happy to revert to my previous version.
There was a problem hiding this comment.
No, leave it. Ideally we'd just get rid of that type entirely but that's work for another day.
schomatis
deleted the
schomatis/directory/sharding/switchToBasic-optimization-1
branch
Extracted from review in #94 (comment).
This ended up being a much cleaner patch than I originally anticipated. Since
go-merkledagprovides an API to add both nodes and links we now add links directly to the node throughAddRawLinkwhile maintaining the currentAddChildAPI (not needing to expand it with a public variant and keeping the plumbingaddLinkChildfunction private) that now converts itself the node to a link (a conversion that would otherwise happen inside the replacedAddNodeLinkcall).