On Wed, Jul 02, 2008 at 02:46:01PM -0700, Dan Price wrote:

> > version.py:
> > 
> >   - line 152: do you want to do any further semantic checking on the
> >     string?  That is, months can't be greater than 12, etc?
> 
> Sigh, that's a big problem.  My concern is that it's going to be rather
> expensive for all the checks we'll need to do.  It may be that it winds
> up being worth storing the timestamp as an integer, since we'll need to
> pay the price to convert to ints during the validity checking.

Yup.  And conversion from integer to string isn't (or shouldn't be) that
expensive.

> As an aside, this led me to discover that we blow up in an ugly way
> today if you try to do e.g.:
> 
> pkg info pkg:/[EMAIL PROTECTED],5.11-0.91:20080613T999999Z
>                                                  ^^^^^^^

Ooh, yeah, yuck.

> My code has the lucky side effect of letting us fail more gracefully,
> but the cost is an overall decrease in safety.  Thoughts?

I'd lean towards just doing the safe thing, even though there aren't a
whole lot of normal paths to get it wrong.

> >   - line 263: What's magic about this?  I don't entirely understand why
> >     build_release is looked at here but not in __lt__ or __gt__, but that's
> >     not magic, per se.> 
> 
> I guess my point was: does anyone understand why build_release is referenced
> here?  I was not intending to check in the XXX, but rather to flag something
> strange.

I think you can get rid of the build_release reference here, since we're
pretty much not using it anywhere, and it doesn't really make much sense
to have it here and not in __gt__ and __lt__.  We can put it in again if
it turns out to be necessary.

Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to