simplefs stub #735
simplefs stub #735
Conversation
|
We should let the KBFS team handle the review. At the very least there's a vendoring protocol to use that should be followed. |
|
Does this need to live inside libkbfs? That package is quite large as is. Typically the interfaces go into their own packages (libfuse, libdokan, ...). So instead of kbfs/libdokan how about kbfs/simplefs? |
|
Makes sense for it to live somewhere else. Also not sure if all these should go in KeybaseServiceBase or what the best alternative may be in Go. Does someone else from kbfs want to weigh in @strib ? Pretty sure some go tests should exercise these soon as one of the next steps. |
|
Also needs rebasing due to conflict. |
|
The tests I made just do local FS copying - probably not that helpful. RE rebasing: I think the source changes to the client branch need to be merged first, not sure we have a thumb yet. |
zanderz
force-pushed
the
zanderz/fsapi_2017.01.26
branch
from
4e34001
to
1d35bf9
Compare
| ) | ||
|
|
||
| // SimpleFS - implement keybase1.SimpleFS | ||
| type SimpleFS struct{} |
There was a problem hiding this comment.
Please add a line like this somewhere in this file so we can assert this implements the expected interface on a re-vendor:
var _ keybase1.SimpleFSInterface = (*SimpleFS)(nil)| require.NoError(t, err) | ||
|
|
||
| listResult, err := sfs.SimpleFSReadList(ctx, opid) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
This will fail if you run this test for real, right? I notice this directory's tests are not yet included in appveyor.yml or Jenkinsfile. I assume that's on purpose until this test will actually pass?
There was a problem hiding this comment.
Forgot about appveyor, good point, but yes, these are expected to fail until am implementation happens. I tried temporarily naming them Example* but the linter didn't like that.
vendor/vendor.json
Outdated
| @@ -192,8 +192,8 @@ | |||
| { | |||
| "checksumSHA1": "90p9joLcyYEYBCMBY2xF/8eYAAM=", | |||
| "path": "github.com/keybase/client/go/protocol/keybase1", | |||
| "revision": "804b64589ed62d7e5530153c4434a2fc0caccbac", | |||
| "revisionTime": "2017-02-08T16:43:02Z" | |||
| "revision": "3e0a0841a41e8fa7f2f12c279e901086d55bee64", | |||
There was a problem hiding this comment.
We've already vendored this in, in current master. So you can drop this vendoring part after a rebase.
zanderz
force-pushed
the
zanderz/fsapi_2017.01.26
branch
from
1d35bf9
to
c1379b4
Compare
Does this look roughly right @maxtaco ? These are the stubs @taruti will help me fill out.