Hi Augie, On Thu, May 20, 2010 at 05:07:22PM -0500, Augie Fackler wrote: > http://github.com/durin42/dulwich now has two branches: > d42-send-fix: the send-pack fix along with some already-passing compat > tests, for 0.6.0 Thanks, this one was merged for 0.6.0. It also appears to be different with a different Git SHA on d42-refactor-clients.
> d42-refactor-clients: the larger client refactor, for after 0.6.0, > pending discussion I'm now looking at this one in more detail. With regard to connect_ssh / SSHVendor I agree it's a good idea to get rid of the current approach. I'll need to fix bzr-git to pass in a single function, but I'm happy to do that. FWIW the reason we had a SSHVendor class was to match what bzr has so I could hook that into it easily. Bazaar has more methods than connect_ssh on SSHVendor, most prominently connect_sftp, which is why it is an object rather than a method. This branch changes GitClient instances to be re-usable - previously they could really only be used once, after which they would have to be discarded, as the server disconnects you after each request. This seems like the right approach to take - it makes it possible for us to support the server keeping the connection open after a request, and it makes it easier for users of the Dulwich API since they don't have to bother with re-initializing GitClient objects. My only concern is with putting can_read on Protocol, is that really necessary ? I'd rather just see _connect()/connect_ssh() return a Protocol instance and a can_read() method. can_read() seems like a mismatch for Protocol to me - it won't tell you whether you can actually do an operation without it blocking. It's just used as a hackish mechanism in GitClient to let it peek whether it should look at the next element in the graph or read more info from the server. Not having Protocol.can_read would also reduce the number of places that are affected by this patch. Cheers, 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
