RESOLVED FIXED 68975
watchlist: Add cross-checks for WatchList once it is filled. 
https://bugs.webkit.org/show_bug.cgi?id=68975
Summary watchlist: Add cross-checks for WatchList once it is filled.
David Levin
Reported 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.
Attachments
Patch (16.36 KB, patch)
2011-09-29 15:36 PDT, David Levin 		eric: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
David Levin
Comment 1 2011-09-29 15:36:21 PDT
Created attachment 109212 [details] Patch
Eric Seidel (no email)
Comment 2 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.
David Levin
Comment 3 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.
Eric Seidel (no email)
Comment 4 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.
David Levin
Comment 5 2011-09-29 22:22:56 PDT
Committed as http://trac.webkit.org/changeset/96390
Note You need to log in before you can comment on or make changes to this bug.