Code review comment for lp://staging/~vila/bzr/308472-info-version-smart-server

Revision history for this message
Andrew Bennetts (spiv) wrote :

The idea seems ok, but:

 - it should guard against the server sending no 'Software version' header. (Unlikely, but possible.)
 - it should not assume repositories have a _client attribute. I expect that will break on local repositories. Instead it should probably use something like repository.bzrdir.root_transport.get_smart_medium() and construct a _SmartClient from that. (Actually, it shouldn't depend on there being a remote repository at all, e.g. consider branch references.)
 - I think the smart server version ought to be reported on its own line, not as part of the "format".

And as Robert says, it would be nice if it worked even without a remote bzrdir of any sort, and it would be good to add tests.

review: Needs Fixing

« Back to merge proposal