[webkit-reviews] review denied: [Bug 81101] sheriffbot should also be addressable with a comma in addition to colon : [Attachment 131836] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Gustavo Noronha (kov)
<gns at gnome.org>'s request for review:
Bug 81101: sheriffbot should also be addressable with a comma in addition to
colon
https://bugs.webkit.org/show_bug.cgi?id=81101

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=131836&action=review


> Tools/Scripts/webkitpy/common/net/irc/ircbot.py:79
> +	   if not
irclib.irc_lower(request[:nickname_length]).startswith(irclib.irc_lower(self.co
nnection.get_nickname())):

What's the point of trimming request to nickname_length if you're going to use
startswith anyway?

> Tools/Scripts/webkitpy/common/net/irc/ircbot.py:84
> +	   if request[nickname_length] == ':':

Won't this crash if len(request) == nickname_length ?

> Tools/Scripts/webkitpy/common/net/irc/ircbot.py:85
> +	       request = request.split(':')

Also, don't we need to use split(":", 1) to avoid breaking in cases like:

sheriffbot: some message with a : character

?