-
Notifications
You must be signed in to change notification settings - Fork 51
Initial implementation #1
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
Conversation
@somebody32 I removed babel because it was slow and got tired of it, the code looks all-right anyway |
@diasdavid would be nice if you could take a look at the initial proposal to be sure I'm on the right way |
var LOCK_PATH = this.LOCK_PATH | ||
|
||
function onFinish (buff) { | ||
lockFile.unlock(LOCK_PATH, function () { |
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.
missing error handling
return this._write(key, content, 'a', cb) | ||
}, | ||
|
||
write: function (key, content, cb) { |
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.
It would be nice to also offer a stream interface
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.
good point, initially I was returning the write stream but it got lost when implementing the locks. Will create a richer API as I move forward.
Seems you are in the right direction :) I like that the interface revolves the several types of data stored in the Repo. It would be good to start having tests for the experiments you are doing, so other people feel confident to PR too. You had some thoughts on how repo should be structured on disk, might wanna share with @whyrusleeping before 0.4.0 release, since 0.4.0 has a different repo structure already, it will be the perfect time for other significant migrations. |
Is there anywhere I can see what's planned in the 0.4.0 release? I've reached a point where I will have to start reverse-engineering the go implementation though since specs and reality don't seem to match (opened ipfs/specs#41) |
@masylum I'm sorry about that, I really need to fix the spec there to I think we can move the nesting to be 3 levels in go-ipfs but some other If anybody wants to help me writing this stuff up it would be sweet.
|
tv left some notes here that may help: https://github.com/ipfs/go-ipfs/blob/master/repo/fsrepo/fsrepo.go#L356 |
I moved forward with the prefix as it is in go-ipfs. If the spec materializes in some other way is easy for me to change it back. Added a I also will need some guidance with the rest since I'm not sure I understand the |
@masylum sounds good to me. re the |
@masylum you have write perms on this repo, mind if you do a PR to a branch in this repo and then a PR to master, so we can work together without having to PR the PR the PR :) |
I went ahead and moved the PR to be a branch on the main repo, so it enables easier collaboration - #2. Closing this one. |
🎩 What? Why?
First prototype to support https://github.com/ipfs/specs/tree/master/repo
Implemented part of
fs-repo
adapter by usingfs-blob-store
, paving the wayfor browser-compatible blob stores (@somebody32 working on it)
👻 GIF
^ streams