Bug 68975 - watchlist: Add cross-checks for WatchList once it is filled.
Summary: watchlist: Add cross-checks for WatchList once it is filled.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: David Levin
URL:
Keywords:
Depends on:
Blocks: 68822
  Show dependency treegraph
 
Reported: 2011-09-27 23:57 PDT by David Levin
Modified: 2011-09-29 22:22 PDT (History)
2 users (show)

See Also:


Attachments
Patch (16.36 KB, patch)
2011-09-29 15:36 PDT, David Levin
eric: review+
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-09-27 23:57:09 PDT
No definition should be allowed to remain if it is unused.

No definition should be referenced if it isn't defined.
Comment 1 David Levin 2011-09-29 15:36:21 PDT
Created attachment 109212 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-09-29 17:59:24 PDT
Comment on attachment 109212 [details]
Patch

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

drive-by-nits.

> Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:32
> +from webkitpy.common.memoized import memoized

Did you intend to use this?

> Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:140
> +        if len(definition_set):

if definition_set: will do approximately the same thing.
Comment 3 David Levin 2011-09-29 19:29:09 PDT
(In reply to comment #2)
> (From update of attachment 109212 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=109212&action=review
> 
> drive-by-nits.
> 
> > Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:32
> > +from webkitpy.common.memoized import memoized
> 
> Did you intend to use this?

I'll remove that. I attempted to use it but found that I couldn't use it for a dictionary.

> 
> > Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:140
> > +        if len(definition_set):
> 
> if definition_set: will do approximately the same thing.

Yep, will change.
Comment 4 Eric Seidel (no email) 2011-09-29 20:43:08 PDT
Comment on attachment 109212 [details]
Patch

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

I'm not sure I 100% follow, but I'm willing to rubber-stamp this.

> Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:111
> +                raise Exception('A rule for definition "%s" is empty, so it should be deleted.' % complex_definition)

I wonder if you want your own Exception subclass.
Comment 5 David Levin 2011-09-29 22:22:56 PDT
Committed as http://trac.webkit.org/changeset/96390