[webkit-reviews] review granted: [Bug 47709] mocktool.MockOptions shouldn't default values to objects : [Attachment 70824] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 47709: mocktool.MockOptions shouldn't default values to objects
https://bugs.webkit.org/show_bug.cgi?id=47709

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

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

Fantastic!  Thanks!

> WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py:46
> +	   options.blocks = True
> +	   options.cc = 'MOCK cc'
> +	   options.component = 'MOCK component'
> +	   options.confirm = True
> +	   options.email = 'MOCK email'
> +	   options.git_commit = 'MOCK git commit'
> +	   options.obsolete_patches = True
> +	   options.open_bug = True
> +	   options.port = 'MOCK port'
> +	   options.quiet = True
> +	   options.reviewer = 'MOCK reviewer'

So this was the minimum list required to get the tests to pass I assume?

> WebKitTools/Scripts/webkitpy/tool/mocktool.py:587
> +class MockOptions(object):
> +    def __init__(self, **kwargs):
> +	   for key, value in kwargs.items():
> +	       self.__dict__[key] = value

Very slick!

You might want to add a comment here to explain why we don't have any default
options here.  Maybe we should?  Or maybe we should have a
WebKitPatchMockOptions which has a bunch of global defaults?  (like port =
None)