-
Notifications
You must be signed in to change notification settings - Fork 160
Fix; queue ordering and errors formatting #2238
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
Conversation
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.
While you're at it, could you please copy-paste and adapt the way we show errors on the current status page? There is a clickable button that expands the errors if there are any. Without such a button, the page layout will probably look terrible if we just dump the error content directly into the table.
@@ -97,6 +98,21 @@ function formatStatus(status: BenchmarkRequestStatus): string { | |||
} | |||
} | |||
|
|||
function hasErrors(errors: Dict<string>) { | |||
// This is a bit odd but if the error object has values the loop will be |
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.
You already use Object.keys(errors).length
in getErrorsLength
, you can use the same here :)
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.
6913885 👍
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.
Thanks!
error_name: error\n
.createdAt
being used in theCompleted At
column (not sure if the column name is the one that needed changing or the value).From the refactor we have lost the ability to have the parent that will be benchmarked appear "above the waterline".