[webkit-reviews] review granted: [Bug 68869] watchlist: Change watchlistparser.py to be class based. : [Attachment 108790] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted David Levin <levin at chromium.org>'s
request for review:
Bug 68869: watchlist: Change watchlistparser.py to be class based.
https://bugs.webkit.org/show_bug.cgi?id=68869

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108790&action=review


Thanks.  I've had to learn this the hard way.  Basically it becomes impossible
to test static module funtions.  Adding them into a class makes it super easy
to have the class hold an Executive/FileSystem/User or other mockable object if
needed.

> Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:33
>  _INVALID_DEFINITION_NAME_REGEX = r'\|'

I bet you want to make these class members now too.

> Tools/Scripts/webkitpy/common/watchlist/watchlistparser_unittest.py:52
> +	   watch_list_parser = WatchListParser()

You could have a setUp which allocates self.parser = WatchListParser() if you
prefer.