[webkit-reviews] review granted: [Bug 221943] [perf.webkit.org] Do not schedule jobs to Buildbot if the last job failed : [Attachment 420482] v2
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 16 15:37:39 PST 2021
dewei_zhu at apple.com has granted Dean Johnson <dean_johnson at apple.com>'s request
for review:
Bug 221943: [perf.webkit.org] Do not schedule jobs to Buildbot if the last job
failed
https://bugs.webkit.org/show_bug.cgi?id=221943
Attachment 420482: v2
https://bugs.webkit.org/attachment.cgi?id=420482&action=review
--- Comment #4 from dewei_zhu at apple.com ---
Comment on attachment 420482
--> https://bugs.webkit.org/attachment.cgi?id=420482
v2
View in context: https://bugs.webkit.org/attachment.cgi?id=420482&action=review
> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:84
> + lastCompletedBuildSuccessful() { return this._lastCompletedBuild == null
|| this._lastCompletedBuild.result() == 0; }
Ditto per https://webkit.org/code-style-guidelines/#zero-comparison
> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:193
> + if ((!this._lastCompletedBuild ||
this._lastCompletedBuild.buildTag() < entry.buildTag()) && entry.hasFinished())
buildTag doesn't guarantee to be indicative of build time, but in Buildbot
case, the larger build tag means more recent
> Websites/perf.webkit.org/tools/js/buildbot-triggerable.js:254
> + if (syncer.isTester() && nextRequest.order() == 0 &&
!syncer.lastCompletedBuildSuccessful())
Nit: per webkit code style guideline
https://webkit.org/code-style-guidelines/#zero-comparison, it should be
`!nextRequest.order()`
Also leave a comment about currently limiting to tester syncer.
More information about the webkit-reviews
mailing list