[webkit-reviews] review granted: [Bug 177225] Update syncing script to be able to build binary for commit set with owned commits. : [Attachment 322162] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 28 20:52:29 PDT 2017


Ryosuke Niwa <rniwa at webkit.org> has granted dewei_zhu at apple.com's request for
review:
Bug 177225: Update syncing script to be able to build binary for commit set
with owned commits.
https://bugs.webkit.org/show_bug.cgi?id=177225

Attachment 322162: Patch

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




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

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

> Websites/perf.webkit.org/public/v3/models/commit-set.js:72
> +    topLevelRepositories() { return
Repository.sortByNamePreferringOnesWithURL(this._repositories.filter((repositor
y) => { return !this.ownerRevisionForRepository(repository); })); }

This is pretty long.
Could you split into multiple lines.
Also, you don't need { return ~ }. You can just do
this._repositories.filter((repository) =>
!this.ownerRevisionForRepository(repository))

> Websites/perf.webkit.org/public/v3/models/commit-set.js:220
> +    topLevelRepositories() { return
Repository.sortByNamePreferringOnesWithURL(this.repositories().filter((reposito
ry) => { return !this.ownerRevisionForRepository(repository); })); }

Ditto.

> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:285
> +		       const requiresBuild =
value.repositoriesToCheck.map(commitSet.requiresBuildForRepository.bind(commitS
et)).some((requiresBuild) => requiresBuild);

Just do repositoriesToCheck.some((commitSet) =>
commitSet.requiresBuildForRepository()) instead.


More information about the webkit-reviews mailing list