D5589: watchman: add the possibility to set the exact watchman binary location

indygreg (Gregory Szorc) phabricator at mercurial-scm.org
Wed Jan 16 16:32:43 EST 2019


indygreg added a comment.


  Overall I like the feature. But the class initialization logic is wonky. That's not your fault: you happen to be wading into a mess. Since it looks like there will be a v2 on this series, I'd encourage you to add some inline comments about the `__init__` mess at the least, or ideally refactor it so the code is cleaner.

INLINE COMMENTS

> __init__.py:567-571
> +        def __init__(sockpath, timeout):
> +            self.sockpath = sockpath
> +            self.timeout = timeout
> +            self.watchman_exe = watchman_exe
> +        return __init__

Why the nested `__init__`?

Perhaps this initialization code could be cleaned up so we are assigning an instance instead of a class to `self.transport`?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5589

To: lothiraldan, #hg-reviewers
Cc: indygreg, mjpieters, mercurial-devel


More information about the Mercurial-devel mailing list