[webkit-reviews] review granted: [Bug 200274] Analysis task page should show build request author and creation time. : [Attachment 375164] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted dewei_zhu at apple.com's request for
review:
Bug 200274: Analysis task page should show build request author and creation
time.
https://bugs.webkit.org/show_bug.cgi?id=200274

Attachment 375164: Patch

https://bugs.webkit.org/attachment.cgi?id=375164&action=review




--- Comment #3 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 375164
  --> https://bugs.webkit.org/attachment.cgi?id=375164
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375164&action=review

> Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js:376
> +	       const requestSummary = `Scheduled${currentGroup.author() ? 'by
"' + currentGroup.author() + '"': ''} at ${currentGroup.createdAt()}`

Need a space between scheduled & by.
Embedding conditions inside a literal expression like this makes impossible to
decipher.
Please split that into a separate statement as in:
const authoredBy = currentGroup.author() ? ' by ' + currentGroup.author() : '';
this.content('request-summary').innerHTML = `Scheduled ${authoredBy} at
${currentGroup.createdAt()}`
Multiple whitespaces are automatically collapsed into one by CSS.