NEW185269
Range bisector should check the commits for repositories without change in specified range.
https://bugs.webkit.org/show_bug.cgi?id=185269
Summary Range bisector should check the commits for repositories without change in sp...
dewei_zhu
Reported 2018-05-03 14:42:12 PDT
Range bisector should check the commits for repositories without change in specified range.
Attachments
Patch (9.94 KB, patch)
2018-05-03 14:53 PDT, dewei_zhu
no flags
Patch (10.12 KB, patch)
2018-05-03 17:03 PDT, dewei_zhu
rniwa: review+
dewei_zhu
Comment 1 2018-05-03 14:53:32 PDT
dewei_zhu
Comment 2 2018-05-03 15:09:18 PDT
Ryosuke Niwa
Comment 3 2018-05-03 16:11:22 PDT
Comment on attachment 339472 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339472&action=review > Websites/perf.webkit.org/ChangeLog:8 > + For the repositories those do not have change in specified range, we still need to use them This is kind of wordy. How about "For repositories without a change in the specified range, > Websites/perf.webkit.org/ChangeLog:9 > + to filter commit sets. You need to explain why we need to do that. In a way, what you've stated so far is "what". We need "why". > Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js:19 > + const topLevelRepositoriesWithCommitHasOrdering = firstCommitSet.topLevelRepositories() This phrase isn't grammatically correct. topLevelRepositoriesWithOrderedCommits? > Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js:666 > + it('should use still check the repository revision even the repository has no change in the range', async () => { should still check or should still use? Can't have two verbs.
dewei_zhu
Comment 4 2018-05-03 17:03:43 PDT
Ryosuke Niwa
Comment 5 2018-05-08 22:55:55 PDT
Comment on attachment 339488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339488&action=review > Websites/perf.webkit.org/ChangeLog:16 > + (async): Remove this? > Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js:35 > + let commits = null; > + if (startCommit !== endCommit) > + commits = await CommitLog.fetchBetweenRevisions(repository, startCommit.revision(), endCommit.revision()); > + else > + commits = [startCommit]; How about just this? cont commits = startCommit == endCommit ? [startCommit] : await CommitLog.fetchBetweenRevisions(repository, startCommit.revision(), endCommit.revision());
Note You need to log in before you can comment on or make changes to this bug.