Added gRPC interface to collect every sketch file #926
Conversation
|
Codecov link is failing again, the rest of the tests are fine. 🤷 |
rpc/commands/commands.proto
Outdated
| // Progress of the downloads of the platforms and libraries files. | ||
| DownloadProgress progress = 2; | ||
| // Description of the current stage of the upgrade. | ||
| TaskProgress task_progress = 3; |
There was a problem hiding this comment.
No idea, I didn't manually change it, it's probably some auto formatting that I missed.
There was a problem hiding this comment.
It's not about the indentation; this is a change from 1 to 2, and from 2 to 3. Just asking though...
There was a problem hiding this comment.
Yes, I noticed it. Still not sure how it did happen. 🤷
commands/instances.go
Outdated
| func SketchLoad(ctx context.Context, req *rpc.SketchLoadReq) (*rpc.SketchLoadResp, error) { | ||
| sketch, err := builder.SketchLoad(req.SketchPath, "") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("Error loading sketch: %v", err) |
There was a problem hiding this comment.
Can we make req.SketchPath as part of the error message?
There was a problem hiding this comment.
Yes, we could but think that some informationg is already in err.
rpc/commands/commands.proto
Outdated
| @@ -58,6 +58,9 @@ service ArduinoCore { | |||
| // Get the version of Arduino CLI in use. | |||
| rpc Version(VersionReq) returns (VersionResp) {} | |||
|
|
|||
| // Returns all files composing a Sketch | |||
| rpc SketchLoad(SketchLoadReq) returns (SketchLoadResp) {} | |||
There was a problem hiding this comment.
[minor]: Why not LoadSketch? (verb + object)
There was a problem hiding this comment.
I've used the same name that's used in the builder package: https://github.com/arduino/arduino-cli/blob/master/arduino/builder/sketch.go#L103
I can change it LoadSketch with no issues by the way, I like the suggestion.
rpc/commands/commands.proto
Outdated
| message SketchLoadReq { | ||
| // Absolute path to single sketch file or a sketch folder | ||
| string sketch_path = 1; | ||
| } |
There was a problem hiding this comment.
This should also include the instance.
message SketchLoadReq {
Instance instance = 1;
// Absolute path to single sketch file or a sketch folder
string sketch_path = 2;
}
There was a problem hiding this comment.
Well, I do not know if it's required, but strange to not having symmetric APIs. @cmaglie, can you please help with this? Thank you!
There was a problem hiding this comment.
I didn't include it because it's actually not used by that command but I guess it costs nothing adding it.
There was a problem hiding this comment.
Works well so far, great simplification for the IDE. Thanks!
|
Fixed those issues you pointed out @kittaakos. |
This reverts commit 21d2533.
Please check if the PR fulfills these requirements
before creating one)
our contributing guidelines
Exposes a gRPC interface to collect all files composing a sketch.
gRPC interface is not exposed.
gRPC consumers have a way to collect all files composing a sketch.
Nope.
Fixes As a gRPC consumer, I want the CLI to collects all the files composing a sketch #837.
See how to contribute