-
-
Notifications
You must be signed in to change notification settings - Fork 398
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 when port or fqbn flags are not set #523
Conversation
0a42661
to
d235a6b
Compare
Hi @federicobond thanks for the PR. The code doesn't compile, can you please go through this doc https://github.com/arduino/arduino-cli/blob/master/CONTRIBUTING.md, make sure tests pass on your computer and then push again? I'll review your code as soon as the tests pass, thanks! |
d235a6b
to
d697109
Compare
d697109
to
f04ff74
Compare
Hi @masci, sorry about that, I was trying to get the integration tests to pass and I botched the latest code changes! I still have some problems with the test match because the error is written to stderr, which is not captured by |
@masci done! |
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.
👍 PR looks great!
|
||
currDownloadDir string | ||
currDataDir string | ||
currSketchbookDir string | ||
) | ||
|
||
type stdOutRedirect struct { | ||
type outputRedirect struct { |
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.
🍰
I just noticed that this does not play along well with the board FQBN configured via the |
This changeset marks the fqbn and port CLI flags as required in the compile and upload commands. The error message is thus changed from the generic:
To the more directly actionable:
Which is followed by the CLI help text.