[webkit-reviews] review denied: [Bug 106184] sheriffbot can't tell me who "kov" is : [Attachment 181468] Patch

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has denied Alan Cutter
<alancutter at chromium.org>'s request for review:
Bug 106184: sheriffbot can't tell me who "kov" is
https://bugs.webkit.org/show_bug.cgi?id=106184

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

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=181468&action=review


> Tools/Scripts/webkitpy/tool/bot/irc_command.py:245
> +	       return '%s: Usage: whois SEARCH_STRING\t// Surround with
double-quotes for exact match eg. "username"' % nick

Don't use "//" for the help string, use parenthesis or something that is
otherwise grammatically correct.

> Tools/Scripts/webkitpy/tool/bot/irc_command.py:248
>	   contributors =
CommitterList().contributors_by_search_string(search_string)

Most of this patch feels like logic that should be pushed into CommitterList
(the "model") rather than done here (the view/controller). The other irc
commands are very thin wrappers around functionality present elsewhere in 
webkitpy.

Also, I'm at least a bit puzzled that, for this specific example, we wouldn't
just return one match for "kov" and leave it at that. Personally I would've
just done a glob-style match across all of the string fields (so "kov" would
match only kov at webkit.org and you'd have to specify "sheriffbot: whois *kov*"
to get a list of other possible matches. I would also be fine with returning
results ranked in some way. The "fuzzy by default but add additional code for
exact matching" feels like overkill to me, but I wouldn't r- the patch just for
that.