/protocol organizational refactoring #183
Conversation
vqhuy
force-pushed
the
refactor-protocol
branch
2 times, most recently
from
f1952bb to
9c39ca2
Compare
vqhuy
force-pushed
the
refactor-protocol
branch
from
9c39ca2 to
84226da
Compare
masomel
force-pushed
the
refactor-protocol
branch
from
84226da to
4d46653
Compare
protocol/directory/directory.go
Outdated
|
|
||
| import ( | ||
| "bytes" | ||
|
|
||
| "github.com/coniks-sys/coniks-go/crypto/sign" | ||
| "github.com/coniks-sys/coniks-go/crypto/vrf" | ||
| "github.com/coniks-sys/coniks-go/merkletree" | ||
| p "github.com/coniks-sys/coniks-go/protocol" |
There was a problem hiding this comment.
Nit pick: We should probably be consistent with how we import some other coniks-go packages. We always import protocol as p which I think is fine, but merkletree sometimes is imported as m (e.g. https://github.com/coniks-sys/coniks-go/blob/master/protocol/consistencychecks.go#L13), and other times (like here) not. I think I've seen this with directory as well (dir vs the full name).
|
@c633 I went ahead and moved around the new test cases I added in #182 to make the tests pass again. There are, however, two tests in consistency_checks.go that no longer pass ( |
|
|
||
| package protocol | ||
| package client |
There was a problem hiding this comment.
@masomel @liamsi @arlolra I would like to have your inputs about this name. It is collided with our application client package (see https://github.com/coniks-sys/coniks-go/pull/183/files#diff-50a5a7c3748e6d839ee365d7f4291a1cR13). A name I think of is verifier. What do you think?
There was a problem hiding this comment.
Hmm. I think the auditor can also be considered a type of verifier, so I don't think this is the best name for this package.
How about we change the client CLI package name? (Btw, we'll have to find a name other than auditor for the auditor CLI, too). Can we call it something like client-bin or client-app?
There was a problem hiding this comment.
What do you think about 4253664?
|
Our error codes are a mess for now. Another PR will follow up. |
vqhuy
changed the title
|
All tests are fixed. PTAL. |
I think yes. |
vqhuy
force-pushed
the
refactor-protocol
branch
from
189e0cb to
46f995f
Compare
|
In the spirit of refactoring: Is there an application layer API that we can refactor from the CLIs? I haven't spent much time looking into this, but it could be useful to have an application layer API for each component in the same way that we have a protocol level API for each. This could then help developers integrate things like a client with their existing applications. What do you think? |
Yes, it is. Actually, every module that doesn't belong to |
vqhuy
changed the title
vqhuy
force-pushed
the
refactor-protocol
branch
from
d24cd99 to
f25a821
Compare
Merge #170 & #182 first.