Bug 198847

Summary: Custom analysis task configurator should allow supplying commit prefix and revision starts 'r'.
Product: WebKit Reporter: dewei_zhu
Component: Tools / TestsAssignee: dewei_zhu
Status: RESOLVED FIXED    
Severity: Normal CC: dewei_zhu, rniwa, simon.fraser
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch rniwa: review+

dewei_zhu
Reported 2019-06-13 19:13:26 PDT
Custom analysis task configurator should allow supplying commit prefix and revision starts 'r'.
Attachments
Patch (21.20 KB, patch)
2019-06-13 19:19 PDT, dewei_zhu 		rniwa: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
dewei_zhu
Comment 1 2019-06-13 19:19:59 PDT
Created attachment 372097 [details] Patch
dewei_zhu
Comment 2 2019-06-17 11:13:09 PDT
rdar://49238080 rdar://49237819
Ryosuke Niwa
Comment 3 2019-06-17 14:17:22 PDT
Comment on attachment 372097 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372097&action=review > Websites/perf.webkit.org/public/include/commit-log-fetcher.php:204 > + $rows = $this->db->query_and_fetch_all('SELECT * FROM commits WHERE commit_repository = $1 AND commit_revision LIKE $2 LIMIT 2', array($repository_id, Database::escape_for_like($revision_prefix) . '%')); This is not right. In the case of subversion revision, we should always check with =. It makes no sense to match r123 against revision 123456 or 456123. > Websites/perf.webkit.org/public/v3/models/commit-log.js:181 > - const data = await this.cachedFetch(`/api/commits/${repository.id()}/${revision}`); > + const data = await this.cachedFetch(`/api/commits/${repository.id()}/${revision}${prefixMatch ? '?prefix-match=true' : ''}`); Just do: this.cachedFetch(`/api/commits/${repository.id()}/${revision}`, prefixMatch ? {prefixMatch} : {}) We could also update cachedFetch to not include the param when it's set to "false" > Websites/perf.webkit.org/server-tests/api-commits-tests.js:401 > + const result = await remote.getJSON('/api/commits/WebKit/210949?prefix-match=false'); Need another test to make sure we default to false when prefix-match is not set.
dewei_zhu
Comment 4 2019-06-17 16:12:25 PDT
Landed in r246522.
dewei_zhu
Comment 5 2019-06-17 16:13:25 PDT
This change is landed in r246522.
Note You need to log in before you can comment on or make changes to this bug.