Bug 67975 - sheriffbot whois should also tell us email addresses
Summary: sheriffbot whois should also tell us email addresses
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-12 17:50 PDT by Ryosuke Niwa
Modified: 2011-09-13 13:50 PDT (History)
2 users (show)

See Also:


Attachments
fixes the bug (3.18 KB, patch)
2011-09-12 17:58 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Updated per abarth's comment (3.38 KB, patch)
2011-09-12 18:14 PDT, Ryosuke Niwa
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2011-09-12 17:50:56 PDT
It's useful when I have to beg someone for fixing builds, bots, etc...
Comment 1 Ryosuke Niwa 2011-09-12 17:58:19 PDT
Created attachment 107117 [details]
fixes the bug
Comment 2 Adam Barth 2011-09-12 17:59:59 PDT
Comment on attachment 107117 [details]
fixes the bug

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

> Tools/Scripts/webkitpy/tool/bot/irc_command.py:194
> +            if len(contributor.emails) and search_string not in contributor.emails:

Upper vs lower case?
Comment 3 Ryosuke Niwa 2011-09-12 18:07:57 PDT
Comment on attachment 107117 [details]
fixes the bug

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

>> Tools/Scripts/webkitpy/tool/bot/irc_command.py:194
>> +            if len(contributor.emails) and search_string not in contributor.emails:
> 
> Upper vs lower case?

Good point. will fix.
Comment 4 Ryosuke Niwa 2011-09-12 18:14:21 PDT
Created attachment 107120 [details]
Updated per abarth's comment
Comment 5 Ryosuke Niwa 2011-09-12 18:15:34 PDT
(In reply to comment #4)
> Created an attachment (id=107120) [details]
> Updated per abarth's comment

On second thought, it might be a good idea to report the correct email address.
Comment 6 Ryosuke Niwa 2011-09-12 18:15:58 PDT
That could be a follow up I guess.
Comment 7 Eric Seidel (no email) 2011-09-13 01:07:47 PDT
Comment on attachment 107120 [details]
Updated per abarth's comment

I guess I would have just used the default contributor __str__, as that prints out their default email anyway?
Comment 8 Ryosuke Niwa 2011-09-13 01:14:17 PDT
(In reply to comment #7)
> (From update of attachment 107120 [details])
> I guess I would have just used the default contributor __str__, as that prints out their default email anyway?

So I think that'll only print out the default email.
Comment 9 Eric Seidel (no email) 2011-09-13 11:54:43 PDT
Comment on attachment 107120 [details]
Updated per abarth's comment

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

> Tools/Scripts/webkitpy/tool/bot/irc_command.py:194
> +            if len(contributor.emails) and search_string.lower() not in map(lambda email: email.lower(), contributor.emails):

You can just use contributor.emails, bool([]) is false anyway.

Why do you need to check the seach_string?  You're trying to avoid printing an email if they gave an email?

Won't that be confusing for "darin" since you won't print "darin@apple.com"?
Comment 10 David Levin 2011-09-13 11:56:03 PDT
Comment on attachment 107120 [details]
Updated per abarth's comment

Seems fine.
Comment 11 David Levin 2011-09-13 11:56:34 PDT
Comment on attachment 107120 [details]
Updated per abarth's comment

Whoops. Undo'ing due to Eric's comments.
Comment 12 Ryosuke Niwa 2011-09-13 11:57:11 PDT
(In reply to comment #9)
> You can just use contributor.emails, bool([]) is false anyway.

Ok, will do.

> Why do you need to check the seach_string?  You're trying to avoid printing an email if they gave an email?
> 
> Won't that be confusing for "darin" since you won't print "darin@apple.com"?

In that case, we will print darin@apple.com.  That's why I manually search through the list of emails.
Comment 13 Eric Seidel (no email) 2011-09-13 12:01:07 PDT
Comment on attachment 107120 [details]
Updated per abarth's comment

With the removal of len(), sure.
Comment 14 Ryosuke Niwa 2011-09-13 13:50:25 PDT
Committed r95042: <http://trac.webkit.org/changeset/95042>