-
-
Notifications
You must be signed in to change notification settings - Fork 404
gRPC client example was converted to a test but that is not what it was intended for #353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
The example code was buried in the repo, if we want that to be a piece of documentation we need to find a better solution than reverting the commit. I'll take care of it. |
@masci I noticed https://github.com/arduino/arduino-cli-client It looks very nice! I do wonder though about the decision to put it in a separate repository. The advantage of leaving the gRPC client example(s) in this repository is it allows the example to be updated in the same commit as the changes to the arduino-cli code that require that update. This means the examples will always match with the version of arduino-cli the user has cloned. I'm sure there are some benefits to putting the example in a dedicated repository, but I thought that the alternative of leaving it in this repo should at least be considered. |
The idea is to provide examples for the same workflow in different languages but if we go down that path I wouldn't clutter this repo with issues, PRs and ultimately code that's related to an example client we only provide for documentation purposes. This is a personal opinion though, so I guess the last word in this regard should come from the project maintainer. If we want to keep things simple, I can easily move back here the |
Seems like it would be easy enough to add compilation testing of the examples to the CI to make sure at least that nothing obviously broke. That's not perfect, but better than nothing.
I don't know what that means. I followed the initial development of the gRPC interface very closely and it seemed to me that the developers did a very good job of updating the client example to reflect changes to the interface. It may not have always happened in the same commit (which I would consider to be best practices), but it looked like something closely resembling best practices to me. I suspect that keeping the example in sync with arduino-cli would be less likely to happen with the example in a separate repository because then the process would likely be:
That extra step is much more likely to be forgotten or skipped. With the example in this repository, even if the developer forgets to update the example, there is still the opportunity for a reviewer to notice this and request that they do so before the PR is merged. |
The documentation should be considered an integral and essential part of the project. Issues and PRs about the documentation are not clutter and this repository is the appropriate place for them. |
This is debatable but I do agree docs should stay close to the code when you want devs to help with keeping them up to date. I should be done with polishing the client example soon, I'll move it here and archive the other repo to avoid confusion. |
c2d9c1b converted the gRPC client example program into a test. As @cmaglie stated on Slack, this file was intended to be an example of how to call the gRPC API. I think this is an important component of the gRPC interface documentation and the conversion to a test makes it less useful as an example. For this reason, I would recommend reverting that part of c2d9c1b.
The text was updated successfully, but these errors were encountered: