Hi Ray Chuan, On Tue, 2010-12-21 at 15:13 +0100, Jelmer Vernooij wrote: > On Tue, 2010-12-21 at 07:05 -0600, Augie Fackler wrote: > > 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.) > No, I hadn't seen that at the time I replied. I think that's a good > suggestion. > > I presume you'd like to override it with whatever mechanism you have in > Mercurial for creating a SSH connection? Do you have a patch for this in progress? If not, I wouldn't mind having a look so it can land before 0.7.0.
Thanks! Jelmer _______________________________________________ Mailing list: https://launchpad.net/~dulwich-users Post to : [email protected] Unsubscribe : https://launchpad.net/~dulwich-users More help : https://help.launchpad.net/ListHelp
