Bug 179047 - OwnedCommitViewer should include the preceding commit.
Summary: OwnedCommitViewer should include the preceding commit.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-31 00:37 PDT by dewei_zhu
Modified: 2017-10-31 00:57 PDT (History)
2 users (show)

See Also:

Attachments
Patch (4.45 KB, patch)
2017-10-31 00:44 PDT, dewei_zhu 		rniwa: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description dewei_zhu 2017-10-31 00:37:33 PDT 
OwnedCommitViewer should include the preceding commit.
Comment 1 dewei_zhu 2017-10-31 00:44:21 PDT 
Created attachment 325426 [details]
Patch
Comment 2 Ryosuke Niwa 2017-10-31 00:57:45 PDT 
Comment on attachment 325426 [details]
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.