Conversation
|
At the current HEAD, the following is done:
You can test out the HTTP by doing |
|
@mappum good progress, this cleans up a lot! 👍 |
|
I guess for now let's just keep outputting CLI-friendly Help-- we can solve that over time once we generate HTTP API docs. |
9ecab5a to
218b1cf
Compare
|
@mappum heads up -- i rebased over master. |
e4bcae3 to
ef32b68
Compare
|
@mappum Is it intended for the calling |
ef32b68 to
44be2e4
Compare
|
@maybebtc Commands have a |
d061e36 to
7434b3a
Compare
|
@mappum cool. as you requested, force-pushed the side-by-side version to the branch under review. |
commands/http/handler.go
Outdated
There was a problem hiding this comment.
@mappum could make it harder to vendor this if we import ipfs-specific packages in the commands lib. Perhaps this root command can be passed in?
func NewHandler(r commands.Command) Handler {
return Handler{Root: r}
}There was a problem hiding this comment.
yeah pass it in. will be good to pull this out and give it to the community as the sweetest command package yet.
|
@jbenet @mappum brought me up to speed on the status of the There are a couple sample commands in place. And there are just a couple remaining framework concerns. These are briefly outlined below. To ease integration, may I suggest a course of action? After the merge, remaining commands can be implemented individually. 🐶 🐕 Dogfooding! Also, I bet it would be really helpful if we'd implement a command (end-to-end) and provide @mappum with feedback. Exercising the various edge-cases will help to iron out the framework and reveal any lingering unknown unknowns. Framework Concerns
CLI Concerns
|
|
In the concerns checklist:
I took care of that, the |
|
@mappum @maybebtc bravo! this clarity on status makes me feel great. LMK which command you'd like me to tackle. Thoughts file args in cli concernI think we could either
|
|
About the file args in CLI: Something I've noticed is that args are almost always just strings, which is why I didn't already make static type definitions for them (on the rare occasion that it is an int or something, the command can convert it itself). Also, adding argument specifications can take away some features, such as variadic args (e.g.
That sounds pretty clean. |
but might be nice if this was done for you. could get rid of lots of implementation bugs if type checking happens on parse
Maybe, not necessarily, you could have something like: cmd := &Command{
Args: []Arg{
Arg{Type: Integer}, // may not be there.
Arg{Type: String, Required: true}, // must have it or fail
Arg{Type: InputFile, Required: true, Variadic: true}, // required = 1 or more
}
Args: []Arg(String)
}And Args slice is considered invalid unless there is exactly one And this arg slice descriptor would error out if any of the If we want to do anything more complicated, can always just get variadic strings and deal with it yourself. (so have full power, but the lib does some lifting for you). |
|
@mappum @maybebtc lmk which commands you'd like me to try to implement tonight |
|
@jbenet |
Heads up @mappum, I'm porting the above to get acquainted. |
…hen parsing commands
…ing Response implementing io.Reader
|
@jbenet @maybebtc |
Now that we have the
commandspackage set up, it's time to move everything over to it. This PR is currently not ready for merge, just setting it up for CR.The basic scope of this PR is as follows:
commandspackage (HTTP RPC server/client, CLI command handling)commandspackage when necessary