[MERGE] Better infrastructure for tracking remote server version in SmartClientMedium

Martin Pool mbp at canonical.com
Thu Jun 12 06:45:58 BST 2008


On Thu, Jun 12, 2008 at 2:35 PM, Andrew Bennetts <andrew at canonical.com> wrote:
> Martin Pool wrote:
>> On Wed, Jun 11, 2008 at 3:05 PM, Andrew Bennetts <andrew at canonical.com> wrote:
> [...]
>> > I don't much like these method names much.  I'm open to suggestions for
>> > improvement.
>>
>> The other thing that was bothering me is that I think the client
>> actually knows the server is in a particular interval of versions: at
>> least X, but no more than Y.  These methods seem a bit confused about
>> that.
>
> Hmm, I suppose that's true.  But we only care about the upper bound, so that's
> all I'm tracking at the moment.

The variable is called "remote is less than" (ie Y) but the method
that tests it is called "remote is at least" (ie X).  One is not
necessarily simply the inverse of the other, unless we know that X ==
Y.

> I guess what I'm saying is the patch I've proposed seems to satisfy all our
> current and anticipated use-cases adequately, and is a simple improvement over
> what we are doing already.  If I change _is_remote_at_least() to a read-only
> property called something like "_remote_version_guess" that callers can do <=
> comparisons on, are you happy with that?

I hadn't realized til we had this thread that it's actually the issue
of ranges above that's the problem.  If you address that somehow I'm
happy for it to be added, and we don't necessarily need to expose the
version.

+    def _remote_is_not(self, version_tuple):

This name is unclear if this is a predicate or an imperative; adding a
verb would help, plus making it clear whether you're setting the upper
or lower bound.  Maybe _remember_remote_is_before?

(I also wonder what will happen in this scheme if we eventually remove
previously supported verbs but maybe yagni...)

-- 
Martin <http://launchpad.net/~mbp/>



More information about the bazaar mailing list