-
Notifications
You must be signed in to change notification settings - Fork 308
"Action failed": highlight the action in red, not the command #1104
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
base: main
Are you sure you want to change the base?
Conversation
|
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D83275549. (Because this pull request was imported automatically, there will not be any future comments.) |
|
While I think the proposed change makes sense, I'm not sure the following is a good idea.
I think it needs to be made obvious that stdout and stderr are empty, since otherwise users can just assume that buck chose to not show this information. If you want it to look nicer, I think it could be changed to something like "stdout: ". If you want to follow up with this, how about let's split this into a separate PR from all the rendering changes? |
|
@scottcao SGTM, thanks! I'll see if I can get to this on Monday. |
We've found that when reading Buck2 error output, users often focus on
the description of the action that failed ("Local command returned
non-zero exit code 1"), which is the first part of the error message
that's rendered in red text. Instead, we would like users to look at the
name of the action that failed, which is rendered in bold text and often
evades notice.
- At the end of a failing Buck2 command, there's a section which recaps
the errors encountered ("BUILD FAILED" "Failed to build
'root//src/Model:Model ...'"). This message is very legible and shows
the failing target clearly, but users don't always see this message at
first. The command may be long-running and the user may be reading
output before it has finished, or the user may simply be reading the
output top-to-bottom.
- The description of the action that failed (currently rendered in red
text) contains supplementary debugging information such as the failing
command's exit code. It also prints the failing command (truncated),
which is somewhat misleading; because Buck2 rules often invoke builds
through layers of wrapper scripts, the failing command is rarely
informative (e.g. a `ghc` failure does not show `ghc`, any wrapper
script filename, or any source filenames in the truncated command).
Although truncating the command is a good tradeoff for brevity, it is
not the most informative part of the error message.
Therefore, this change makes small adjustments to the styling of action
errors:
- The "Action failed" headline is rendered in bold red text, rather than
bold white text. This helps draw the user's attention to the name of
the action that failed, rather than the command that failed. This is
the most important part of the change.
- The reason ("Local command returned non-zero exit code 1") is still
rendered in dark red text, as it provides context for _how_ the action
failed.
- The "Reproduce locally" line is rendered in white text, rather than
dark red text.
baec5cc to
c40e03b
Compare
|
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D83275549. (Because this pull request was imported automatically, there will not be any future comments.) |
|
I made the requested changes like 2 months ago and forgot to push them. Ooops! @scottcao PTAL :) |
TL;DR: See the end of the description for before/after screenshots. In "Action failed" errors, render the action ID in bold red (not bold white), and render the "Reproduce locally" in white, not dark red. This makes it clear which action failed, which tends to be more useful than the (truncated) command itself.
We've found that when reading Buck2 error output, users often focus on the description of the action that failed ("Local command returned non-zero exit code 1"), which is the first part of the error message that's rendered in red text. Instead, we would like users to look at the name of the action that failed, which is rendered in bold text and often evades notice.
At the end of a failing Buck2 command, there's a section which recaps the errors encountered ("BUILD FAILED" "Failed to build 'root//src/Model:Model ...'"). This message is very legible and shows the failing target clearly, but users don't always see this message at first. The command may be long-running and the user may be reading output before it has finished, or the user may simply be reading the output top-to-bottom.
The description of the action that failed (currently rendered in red text) contains supplementary debugging information such as the failing command's exit code. It also prints the failing command (truncated), which is somewhat misleading; because Buck2 rules often invoke builds through layers of wrapper scripts, the failing command is rarely informative (e.g. a
ghcfailure does not showghc, any wrapper script filename, or any source filenames in the truncated command).Although truncating the command is a good tradeoff for brevity, it is not the most informative part of the error message.
Therefore, this change makes small adjustments to the styling of action errors:
The "Action failed" headline is rendered in bold red text, rather than bold white text. This helps draw the user's attention to the name of the action that failed, rather than the command that failed. This is the most important part of the change.
The reason ("Local command returned non-zero exit code 1") is still rendered in dark red text, as it provides context for how the action failed.
The "Reproduce locally" line is rendered in white text, rather than dark red text.
The "stdout:" and "stderr:" sections are only shown if they are non-empty.
Before
After