Bug 224496 – Support Commit revision identifier in the Chart

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zhifei Fang]

Support Commit revision identifiter in the Chart

[Zhifei Fang]

Created attachment 425889 [details]
Patch

Comment on attachment 425889 [details]
Patch

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

r=me

> Websites/perf.webkit.org/ChangeLog:7
> +        In the measurement-set, we include the commit info but not the commit revision identifier column.
> +        Make the API returen the commit revision identifier as part of the commit data.

Nit: this should below `Reviewed by section`
Typo returen -> return

> Websites/perf.webkit.org/server-tests/api-measurement-set-tests.js:501
> +         assert.deepStrictEqual(format(response['formatMap'], currentRows[1]), {

Nit: this indentation doesn't look right.

[Zhifei Fang]

Created attachment 425910 [details]
Patch

[Zhifei Fang]

Created attachment 425938 [details]
Patch

[Zhifei Fang]

Created attachment 425946 [details]
Patch

[Chris Dumez]

Comment on attachment 425946 [details]
Patch

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

> Websites/perf.webkit.org/ChangeLog:3
> +        Support Commit revision identifiter in the Chart

typo: identifiter

[Zhifei Fang]

Created attachment 425947 [details]
Patch

Comment on attachment 425947 [details]
Patch

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

r=me

> Websites/perf.webkit.org/browser-tests/customizable-test-group-form-tests.js:96
> +        // click Radio button will enqueue to render, we will get a fresh input element

Lower case R for `Radio`?

> Websites/perf.webkit.org/browser-tests/customizable-test-group-form-tests.js:109
> +        // customizableTestGroupForm will use commitSet.updateRevisionForOwnerRepository to update the revision, need to await this to see the input value get updated to 210948

need to await this to see the input value get updated to 210948 -> need to await this to see the input value *gets* updated to 210948

[Ryosuke Niwa]

Comment on attachment 425947 [details]
Patch

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

>> Websites/perf.webkit.org/browser-tests/customizable-test-group-form-tests.js:96
>> +        // click Radio button will enqueue to render, we will get a fresh input element
> 
> Lower case R for `Radio`?

I don't think this comment is needed.

> Websites/perf.webkit.org/public/api/measurement-set.php:211
> +        // e.g. {"(<commit-id>,<repository-id>,<revision>,<revision_identifier>,<order>,\"2012-10-16 14:53:00\")","(<commit-id>,<repository-id>,<revision>,<revision_identifier>,<order>,)",
> +        // "(<commit-id>,<repository-id>,<revision>,<revision_identifier>,,)", "(<commit-id>,<repository-id>,<revision>,<revision_identifier>,,\"2012-10-16 14:53:00\")"}

Use - instead of _.

[Zhifei Fang]

Created attachment 426056 [details]
Patch

[Zhifei Fang]

Created attachment 426067 [details]
Patch for landing

[EWS]

Committed r275993 (236545@main): <https://commits.webkit.org/236545@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 426067 [details].

[Radar WebKit Bug Importer]

<rdar://problem/76681144>