-
-
Notifications
You must be signed in to change notification settings - Fork 371
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
Improve error message due to missing FQBN in upload/compile commands #540
base: master
Are you sure you want to change the base?
Conversation
0bce37c
to
b1a8581
Compare
b1a8581
to
0c3c91a
Compare
Hi @federicobond, I gave a look at your PR, good job! I was wondering if you want to modify your PR to try another approach for giving better error info to our users: We moved recently the fqbn / port / sketch folder checks a level below in the To maintain a "Single Responsibility Principle" we think it could be better to:
WDYT? Thanks for your contribution and happy coding! |
Hi, thank you for the feedback, @rsora! One problem I see with that approach is that the error type would not be serialized through the gRPC interface, right? The error message string would, but its type would be lost in the translation. |
I do get your point about the Single Responsibility Principle though, and I am similarly bothered by the duplication we see here, but it's not at all uncommon to have this sort of duplication in other client/server architectures. Sometimes clients are able to provide richer feedback and developers can take advantage of that, which usually requires some duplication of logic. Sometimes they are not, but it's important that users still receive some feedback from the backend. Edit: we could do something like this instead. |
Hi @federicobond, I ended up having a lot of Additionally, we cannot use the So, if we need to continue on this path, I guess we should define specific error type alias for both
What do you think about implementing a POC for this approach? This way we can continue the discussion having some code to look at. Thanks for your suggestion! |
Sure! I can try a POC of that. It will probably happen in the weekend though, feeling exhausted these days during the weeks. |
No description provided.