[webkit-reviews] review granted: [Bug 52284] Split Host object off from WebKitPatch for easier re-use : [Attachment 78669] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 52284: Split Host object off from WebKitPatch for easier re-use
https://bugs.webkit.org/show_bug.cgi?id=52284

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78669&action=review

We should probably rename all the tool variables / properties to host.

> Tools/Scripts/webkitpy/common/host.py:30
> +# Command objects expect to have a self.tool which provides these
interfaces.

Command objects aren't a concept that make sense in common.  I'd just remove
this comment.

> Tools/Scripts/webkitpy/common/host.py:44
> +	   self.bugs = bugzilla.Bugzilla()
> +	   self.buildbot = buildbot.BuildBot()
> +	   self.executive = executive.Executive()

Why don't we just import these names directly instead of using this goofy
syntax?

> Tools/Scripts/webkitpy/common/host.py:81
> +    def command_completed(self):
> +	   if self._irc:
> +	       self._irc.disconnect()

This probably needs a more semantic name like disconnect_from_irc

> Tools/Scripts/webkitpy/tool/main.py:36
> +from webkitpy.common.host import Host

Shouldn't this be alphabetized like everything else?