-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
refactor(core): NewIPFSNode constructor #538
Conversation
@briantigerchow this is great stuff! very much needed. |
return nil, err | ||
} | ||
return uio.NewDagReader(nodeCatted, catterdag) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is beautiful. and yes, +1 on free functions. btw, lots of it can probably be abstracted into an interface (i.e. cat
shouldnt need to know what a path.Resolver
is, and instead could call ipfs.Resolve(path)
)
❤️ want to merge this in asap. |
(btw build err) |
About to rebase on master and fixup the build error. Responding to your comments here since they'll disappear after the rebase:
For the free functions, would you prefer to see them in another package or here in
It originally used the
doh! |
ccb08af
to
e3ac6af
Compare
Another package.
interesing-- any idea why? btw, tests can now use |
e9c2c8f
to
b0849fa
Compare
@briantigerchow LGTM, merge this in if you're ready. |
b0849fa
to
a7f9587
Compare
NB: all commits (from master) pass tests up to this point. fix: squashme into use core constructor
fix: remove merkledag import may need to squash this commit into the merkledag move commit
…n instantiated this is a dirty hack
a7f9587
to
63c0d41
Compare
refactor(core): NewIPFSNode constructor
RFC/RFCR
This PR is a gradual refactor of the
core.IPFSNode
constructor.Primary objectives:
Routing
pluggable in a way that doesn't cause a complexity explosionBefore
After
The old signature can be emulated as follows:
The crux of
ConfigOption
is that it allows for any number of custom configuration options. It takes ideas from Pike's self-referential functions, but is tailored to our specific needs and constraints (we have a lot of these).http://commandcenter.blogspot.nl/2014/01/self-referential-functions-and-design.html
Some type signatures I've considered so far:
I've hit a point in the implementation where it is no longer possible to share more of the constructor code without making tough trade-offs. This is a good time to invite others into the design process.
(big refactor with lots of commits but all commits should pass all tests)