-
-
Notifications
You must be signed in to change notification settings - Fork 398
[breaking] Fix board attach CLI command, and remove gRPC API #1930
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
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.
- There is this test to remove:
board_test.go:452:
Error Trace: /home/runner/work/arduino-cli/arduino-cli/internal/integrationtest/board/board_test.go:452
Error: Received unexpected error:
exit status 1
Test: TestBoardAttachWithoutSketchJson
--- FAIL: TestBoardAttachWithoutSketchJson (2.50s)
- you must run a
go mod tidy
on all sub-projects to fix all the internal go.mod/go.sum - thinking about go.mod, we should also be able to get rid of this dependency:
github.com/arduino/board-discovery
so running ago mod tidy
on the main repo should show it.
Ah you already did it most of it, LOL. I should reload the page before starting a review :-) |
I just pushed it really, you would have missed it in any case. 😅 By the way not sure about the Regarding |
I see, I think I forgot to commit the |
I see dependencies checks are failing but I can't manage to run |
Here you go Silvano: silvanocerza#5 |
Thanks @per1234, greatly appreciated. :) |
Hello @silvanocerza, thanks a lot for this PR! Today with @per1234 @ubidefeo @cmaglie we spoke about this. In the end we decided to not completely deprecate the command, mainly because the users are using this and overall it only needs some love.
|
Sorry, no time to completely rework the command like that. |
55eaa74
to
230ecc4
Compare
I've pushed the implementation, but now I have my own review pending... I can approve the my changes by myself but doesn't look like a good plan mmmmmhhhh... |
be7e651
to
b0375f0
Compare
This comment was marked as outdated.
This comment was marked as outdated.
95729fc
to
9161a24
Compare
613827b
to
e9460fd
Compare
1a798d8
to
7e95d55
Compare
Co-authored-by: Cristian Maglie <[email protected]>
Co-authored-by: per1234 <[email protected]>
Co-authored-by: per1234 <[email protected]>
This change is going into 0.30.0
f8e0dde
to
c946d0b
Compare
Please check if the PR fulfills these requirements
See how to contribute
before creating one)
our contributing guidelines
UPGRADING.md
has been updated with a migration guide (for breaking changes)What kind of change does this PR introduce?
Remove an unused neglected broken feature.
What is the current behavior?
board attach
CLI command andBoardAttach
gRPC interface command are both broken and neglected. Their behaviour is unclear and buggy.What is the new behavior?
Remove
board attach
CLI command andBoardAttach
gRPC interface command and all their usages from the project.Does this PR introduce a breaking change, and is titled accordingly?
Yes and yes.
Other information
Closes #1925.