-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix: let the upload run when no board is selected #10
Conversation
IDE2 must not disable the upload from the UI when no board is selected. Otherwise, IDE2 does not receive the `no FQBN` error from the CLI and cannot cannot suggest selecting a board via the `Tools` > `Board` menu. Closes arduino/arduino-ide#1714 Signed-off-by: Akos Kitta <[email protected]>
Unable to locate .performanceTestingBot config file |
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information |
Processing PR updates... |
Check out the playback for this Pull Request here. |
Thanks @2lambda123 for opening this PR! For COLLABORATOR only :
|
Their most recently public accepted PR is: #9 |
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.
@2lambda123
Thank you for your contribution to this repository! We appreciate your effort in opening pull request.
Happy coding!
Your organization has reached the subscribed usage limit. You can upgrade your account by purchasing a subscription at Stripe payment link Disclaimer: This comment was entirely generated using AI. Be aware that the information provided may be incorrect. Current plan usage: 156.20% Have feedback or need help? |
PR Details of @2lambda123 in arduino-arduino-ide :
|
Warning Rate Limit Exceeded@2lambda123 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 2 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
@2lambda123
Thank you for your contribution to this repository! We appreciate your effort in closing pull request.
Happy coding!
async checkUserFieldsDialog(forceOpen = false): Promise<boolean> { | ||
const key = this.selectedFqbnAddress(); | ||
if (!key) { | ||
return false; | ||
// Let the upload continue without an FQBN and the CLI fail instead of disabling the upload from IDE. | ||
// https://github.com/arduino/arduino-ide/issues/1714 | ||
return true; |
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.
The decision to allow the upload to continue without an FQBN (Fully Qualified Board Name) and rely on the CLI to fail might lead to a less intuitive user experience. Users might not understand why their upload failed if the error messages from the CLI are not clear or are not properly relayed back to the IDE's user interface.
Recommended Solution: Consider implementing a more user-friendly error handling mechanism that checks for the presence of an FQBN before attempting the upload. If the FQBN is missing, provide a clear and informative error message directly in the IDE, guiding the user on how to resolve the issue. This approach can help in reducing confusion and improving the overall user experience.
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.
Hey @2lambda123 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
return false; | ||
// Let the upload continue without an FQBN and the CLI fail instead of disabling the upload from IDE. | ||
// https://github.com/arduino/arduino-ide/issues/1714 | ||
return true; |
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.
suggestion (code_refinement): Consider handling potential CLI failures more gracefully.
Allowing the CLI to fail without prior checks might lead to less user-friendly error handling. It could be beneficial to implement a more graceful fallback or error message to improve the user experience.
return true; | |
if (!fqbn) { | |
console.error("FQBN is missing. Upload cannot proceed."); | |
return false; | |
} | |
return true; |
IDE2 must not disable the upload from the UI when no board is selected. Otherwise, IDE2 does not receive the
no FQBN
error from the CLI and cannot cannotsuggest selecting a board via the
Tools
>Board
menu.Closes arduino/arduino-ide#1714
Motivation
Change description
Other information
Reviewer checklist