monotone-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Monotone-devel] nvm.dates


From: Stephen Leake
Subject: Re: [Monotone-devel] nvm.dates
Date: Sat, 25 Oct 2008 13:33:59 -0400
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (windows-nt)

Markus Wanner <address@hidden> writes:

> I'm considering nvm.dates ready to land on mainline. All it does is
> using a 64 bit integer to represent dates internally, so that date_t now
> provides comparison and difference operators on these timestamps. It
> does not alter the textual representation in the database nor for
> netsync.

It uses an unsigned 64 bit integer, which makes subtracting dates
interesting. 

Hmm. You have:

s64
date_t::operator -(date_t const & other) const
{
  return d - other.d;
}

So that makes subtracting dates ok.

But what is the rationale for an unsigned date type in the
first place?

Hmm. you use -1 (actually 0xFFFF...LL) as an invalid date. And you
have a check on the maximum date, with an implied minimum of zero.

You could just do those checks in the date to string functions.

But I guess using separate types for 'date' and 'date difference'
makes things simpler.

So I think keeping date unsigned is ok, but please document this
rationale in dates.hh.

There are unit tests for the new functions.

This change certainly makes it easier to add operations on dates in
the future, which we might want to do. I can see that some 'import
from other CM tool' functions might want to check dates. 

I see no reason to object to landing it on mainline.

> As there seems to be a majority voting against further checking of the
> date cert information, I'm not continuing my efforts into that
> direction. Instead I've turned that portion of code into an additional
> "--full" helper option for "mtn db info", which now displays some
> statistic analysis of those date certs, see below for examples - no
> warnings, no further checking, only "db info" is affected. I've
> committed that into a separate branch nvm.dates.statistics. Please
> review as well, I consider that to be ready to land on mainline, too.

I see no reason to object to landing this on mainline either. Some of
the stats could be useful; frequency of checkins might correlate with
something :).


-- 
-- Stephe




reply via email to

[Prev in Thread] Current Thread [Next in Thread]