[webkit-reviews] review granted: [Bug 179047] OwnedCommitViewer should include the preceding commit. : [Attachment 325426] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 31 00:57:45 PDT 2017


Ryosuke Niwa <rniwa at webkit.org> has granted dewei_zhu at apple.com's request for
review:
Bug 179047: OwnedCommitViewer should include the preceding commit.
https://bugs.webkit.org/show_bug.cgi?id=179047

Attachment 325426: Patch

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




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

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

> Websites/perf.webkit.org/public/v3/components/commit-log-viewer.js:39
> +	   let fetchSingleCommit = !precedingRevision || precedingRevision ==
lastRevision;

Use const.

> Websites/perf.webkit.org/public/v3/components/commit-log-viewer.js:59
> +		   this._precedingCommit = precedingCommit[0];

You should check this._fetchingPromise != promise here to avoid showing results
for the old range.

> Websites/perf.webkit.org/public/v3/components/commit-log-viewer.js:88
> +	   let commits_to_process = commits && precedingCommit &&
precedingCommit.ownsCommits() ? [precedingCommit].concat(commits) : commits;

Use camelCase. In fact, I think we can just assign directly to commits in the
case we have precedingCommit.


More information about the webkit-reviews mailing list