-
Notifications
You must be signed in to change notification settings - Fork 49
Conversation
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.
Few spelling fix suggestions and opening discussion on couple style related matters we should probably align on in general.
Except for command argument structure, the design & code otherwise LGTM 👍
@@ -0,0 +1,28 @@ | |||
[package] | |||
name = "gevulot-cli" |
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.
I've been thinking about the name. cli
as such feels a bit wrong when the gevulot
itself already provides bunch of functionality that the node operator needs, but at the same time I don't have good suggestions. client
is not really the right one, as there's the JSON-RPC client library already. devctl
or something along that side could maybe work, but that also feels a bit off.
I'd suggest that we go with the gevulot-cli
but try to think about better name for this, to best communicate its functionality & purpose.
Also: We might want to extract this into its own repository at some point, WDYT?
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.
For the name by exp I know I'm not very good, so I take what it's done in other BC. But you can define any name, I don't really have any good idea.
I agree, it's better if the tool have its own repo because update and release rhythm and their consequences are not the same for the node and the client.
More generally, I agree with the Rust idiomatic's way to define crate: define small crate. The drawback is that it add complexity when you have a lot of crate not published in crates.io because you've to manage all the specific commit for all crates you use. It can become tedious when all crates move fast before the first release.
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
All check pass, I can merge? |
I think there are still pending typos open in the diff side or is my GitHub view somehow not showing the latest? Most notable there are couple |
Yes, they were hidden by github. I didn't see. I thought that solved issue were hidden and not opened. I look at them. |
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Co-authored-by: Tuomas Mäkinen <[email protected]>
Add a cli tool:
Deploy prover/verifier
Exec program workflow.