[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