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