-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
internal: Show more project building errors to the user #11960
Conversation
@@ -94,17 +94,17 @@ impl WorkspaceBuildScripts { | |||
by_id.insert(workspace[package].id.clone(), package); | |||
} | |||
|
|||
let mut callback_err = None; |
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.
We were never using this "error case" here, but once set we stop working through the build script output, so this might be the cause of #9720 not working for people without any apparent reason?
// deliberately enable all `error` logs if the user has not set RA_LOG, as there is usually useful | ||
// information in there for debugging | ||
logger::Logger::new(log_file, filter.as_deref().or(Some("error"))).install()?; |
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.
Any arguments against this? The way this PR is structured now is that we do small error popups for build errors and others that only appear on project load and put the bulkier information into the server logs as most notification UIs (or maybe really only VSCode who knows) don't handle big messages that well.
☔ The latest upstream changes (presumably #11956) made this pull request unmergeable. Please resolve the merge conflicts. |
@bors r+ |
📌 Commit b23b276 has been approved by |
☀️ Test successful - checks-actions |
Should help out with #9720
Fixes #11223