[webkit-reviews] review denied: [Bug 36253] First cut at SheriffBot : [Attachment 50978] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 17 17:43:01 PDT 2010


Eric Seidel <eric at webkit.org> has denied Adam Barth <abarth at webkit.org>'s
request for review:
Bug 36253: First cut at SheriffBot
https://bugs.webkit.org/show_bug.cgi?id=36253

Attachment 50978: Patch
https://bugs.webkit.org/attachment.cgi?id=50978&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
I think IRC connection should be explicit.  It's "dangerous".  It should just
return None until someone has explicitly connected.

Or you coudl have an ensure_irc_connected() method, which makes it clear that
it does work.  "irc()" could leave me in some sort of false impression that it
doesn't do any work, or certainly doesn't connect me to anything.

I think if we have some explicit connect method it should take a
username/host/channel, as whoever is calling it will have that information.

I worry that the name "finalize" is overloaded.  Maybe we should have some
custom name.  I think you mentioned "command_completed()" in person.   The
default implementation could pass.


More information about the webkit-reviews mailing list