Make the error on other task message red #404
Conversation
Will make it easier to spot it in the output. Partially fixes science-computing#57 Signed-off-by: Nico Steinle <nico.steinle@atos.net>
There was a problem hiding this comment.
Looks fine to me but you shouldn't use Partially fixes #57 in the commit message if the issue should remain open (which I assume it should if this only partially fixes it) as that would close it automatically (s. https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests for details).
You should still reference #57 though - just make sure there isn't a keyword that closes the issue right in front of it ;)
Does this really help with #57 though? That issue is about which job/dependency triggered the "error on other task" and I though we didn't want to color these inherited/propagated error messages red (for "error occurred" it should be fine and for "error on other task" yellow/orange/etc. could perhaps make sense?).
And is it intentional that you use your Atos email address in the commit metadata and trailer?: Signed-off-by: Nico Steinle <nico.steinle@atos.net>
| @@ -563,9 +563,9 @@ impl<'a> Drop for JobTask<'a> { | |||
| // If there are dependencies, the error is probably from another task | |||
| // If there are no dependencies, the error was caused by something else | |||
| let errmsg = if self.jobdef.dependencies.is_empty() { | |||
| "error occurred" | |||
| "error occured".red() | |||
There was a problem hiding this comment.
Why change "occurred" to "occured"?
https://writer.com/blog/occurred-occured/
(I (hopefully) fixed that as part of 53de218...)
| } else { | ||
| "inherited" | ||
| "error on other task".red() |
There was a problem hiding this comment.
PS: Told you so (#374 (comment)) :P (SCNR :D)
@christophprokop: Can we properly decide on the message and color once and for all (ideally xD)?
|
(because it's red (like in occured)) No, it was an old commit and disregarded all changes made in since the commit was created. |
|
output much better readable since 71b1921 |
Will make it easier to spot it in the output.
Partially fixes #57
This is an older commit. CP tested it, was fine