[Webkit-unassigned] [Bug 153624] Add code to parse the git branches out of the Trac RSS feed

[bugzilla-daemon at webkit.org]

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

Daniel Bates <dbates at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #270426|review?                     |review+
              Flags|                            |

--- Comment #6 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 270426
  --> https://bugs.webkit.org/attachment.cgi?id=270426
Patch

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

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Trac.js:169
> +        var gitBranches = doc.evaluate("./branches", commitElement, null, XPathResult.STRING_TYPE).stringValue;

I am assuming that doc.evaluate("./branches", commitElement, null, XPathResult.STRING_TYPE) always returns an object. We should add a test case for an entry that does not have <branches>.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:54
> +    client.open('GET', 'test-fixture-git-trac-rss.xml', false);

Nit: We use single quoted string literals here and double quoted string literals below. We should pick one quoting style for string literals and stick with it.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:58
> +    client.onreadystatechange = function () {
> +        if (client.readyState === client.DONE)
> +            this.trac._loaded(client.responseXML);
> +    }.bind(this);

Nit: We're underutilizing the the onreadystatechange listener. Since we only care if the load completed successfully, we could register a onload listener. See <https://xhr.spec.whatwg.org/#event-handlers> for a list of the event handlers that can be registered on XMLHttpRequest.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160201/4269fe9a/attachment.html>