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

[lothiraldan (Boris Feld)]

lothiraldan added inline comments.

INLINE COMMENTS

> indygreg wrote in __init__.py:567-571
> 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`?

I see several ways to refactor the code around:

- Pass the watchman_exe parameter to all Transport class even if they don't need it. The `CLIProcessTransport` is already ignoring the timeout attribute.
- Pass a `functools.partial` of `CLIProcessTransport.__init__`.
- Pass the client instance to the transport, which will likely results in a memory leak due to the circular reference.
- Store an instance as `client.transport` but that would requires to move passing the sockpath and update the transport to manage a `ready` state, which would be false until they get all the needed parameters.

I have a small preference for solution 1). Solution 4) seems the most heavyweight

REPOSITORY
  rHG Mercurial

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

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