[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