dulwich-users team mailing list archive

[Augie Fackler <durin42@xxxxxxxxx>]

On Dec 21, 2010, at 4:22 AM, Jelmer Vernooij wrote:
> 
> On Tue, 2010-12-21 at 12:46 +0800, Tay Ray Chuan wrote:
>> Allow the vendor to be specified on the client class. That way, when
>> users wish to use a separate vendor, they can just subclass
>> SSHGitClient, instead of replacing (globally) client.get_ssh_vendor.
>> ---
>> dulwich/client.py                   |    7 +++----
>> dulwich/tests/compat/test_client.py |   25 +++++++++++++------------
>> 2 files changed, 16 insertions(+), 16 deletions(-)
>> 
>> diff --git a/dulwich/client.py b/dulwich/client.py
>> index 2b69852..e89879f 100644
>> --- a/dulwich/client.py
>> +++ b/dulwich/client.py
>> @@ -338,12 +338,11 @@ class SSHVendor(object):
>>                                 stdout=subprocess.PIPE)
>>         return SubprocessWrapper(proc)
>> 
>> -# Can be overridden by users
>> -get_ssh_vendor = SSHVendor
>> -
>> 
>> class SSHGitClient(GitClient):
>> 
>> +    ssh_vendor_class = SSHVendor
>> +
> My main concern is about keeping compatibility with the Bazaar SSHVendor
> interface - both so bzr-git can easily hook it in, but also to keep the
> option open of stealing that code easily in the future. This patch
> doesn't change the SSHVendor class, so that's not really an issue here. 
> 
> I'm happy to merge this but would prefer to keep calling this variable
> get_vendor as ideally it selects a vendor (openssh, sshcorp, plink, lsh,
> paramiko, etc) and returns an instance of that rather than being
> hardcoded to a single class. 

I won't bikeshed what to call it, but IMO we need to not remove get_ssh_vendor in the same version of dulwich that we move to some other interface. Did you see my feedback on this patch? Do you have any thoughts on that? (I wanted to make it possible for clients to not *have* to implement bzr's ssh vendor scheme if that's undesirable.)

> 
> We should also document in NEWS that get_ssh_vendor has moved.
> 
> Cheers,
> 
> Jelmer