[Webkit-unassigned] [Bug 190863] Customizable test group form should not reset manually edited commit value sometimes.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=190863

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

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

> Websites/perf.webkit.org/ChangeLog:7
> +        After changing the radio button and manually editing the commit value, commit value should not be reset
> +        while changing the name of the test group.

Why don't we add a test for this?
And let's split this patch into two since these two changes are pretty complex.

> Websites/perf.webkit.org/ChangeLog:10
> +        Reviewed by NOBODY (OOPS!).

This line should appear before the long description bug after the bug link.

> Websites/perf.webkit.org/ChangeLog:22
> +        (CustomizableTestGroupForm.prototype._constructRevisionRadioButtons):

Let's add a comment saying that this code change is the one which fixes the toggling A/B would not update the commit set.

> Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js:20
> +        this._originalCommitSetMap = map;

How about uncustomizedCommitSetMap to match the terminology?

> Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js:258
> +                commitSetMap.get(columnLabel).updateRevisionForOwnerRepository(repository, revisionEditor.value).catch(
>                      () => revisionEditor.value = '');

This needs to be updated to revert to the old value.
We should probably warn the user that the specified revision doesn't exist with a nice error message.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20181106/401439c2/attachment.html>