Bug 69303 - watchlist: Don't add the same message to a bug more than once.
Summary: watchlist: Don't add the same message to a bug more than once.
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: David Levin
URL:
Keywords:
Depends on: 69308
Blocks: 68822 69323
  Show dependency treegraph
 
Reported: 2011-10-03 16:12 PDT by David Levin
Modified: 2011-10-05 02:22 PDT (History)
2 users (show)

See Also:


Attachments
Patch (8.73 KB, patch)
2011-10-03 19:17 PDT, David Levin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Levin 2011-10-03 16:12:34 PDT
Right now the watchlist will run once per attachment and that may result in spammy messages.
Comment 1 Adam Barth 2011-10-03 16:32:40 PDT
One approach is to look at who is CCed on the bug already.  You can also scan the comments for whatever message you would spam.
Comment 2 David Levin 2011-10-03 16:42:14 PDT
(In reply to comment #1)
> One approach is to look at who is CCed on the bug already.  You can also scan the comments for whatever message you would spam.

I think we're fine on the cc list as I tried it out :) and I adding someone to the cc list who is already there seems to be a no-op.

I only care about the text but since I need to check the text (which requires fetching the bug), I might as check the cc as well to avoid making the server do unnecessary work.
Comment 3 David Levin 2011-10-03 19:17:16 PDT
Created attachment 109573 [details]
Patch
Comment 4 Adam Barth 2011-10-04 22:16:49 PDT
Comment on attachment 109573 [details]
Patch

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

> Tools/Scripts/webkitpy/tool/mocktool.py:311
> -        return Bug(self.bug_cache.get(bug_id), self)
> +        return Bug(self.bug_cache.get(int(bug_id)), self)

Yeah, the issue of when bug ID are numbers and when they are strings is tricky.  We didn't realize it would be a problem until it was too late.  We're more careful in new objects.

> Tools/Scripts/webkitpy/tool/steps/applywatchlist.py:37
> +def is_message_in_comments(bug, message):

Module private functions start with _ by convention.  This seems like it should be a method on bug though.

> Tools/Scripts/webkitpy/tool/steps/applywatchlist.py:61
> +            messages = [message for message in messages if not is_message_in_comments(bug, message)]

You can do this more easily with the filter function.
Comment 5 David Levin 2011-10-05 02:22:48 PDT
Fixed all (and added a simple unit test for the new bug method).

Committed as http://trac.webkit.org/changeset/96685