Zeitgeist Framework

TimeRange.always() is inconsistent

Bug #614295 reported by Michal Hruby
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Zeitgeist Framework
Fix Released
Medium
Seif Lotfy

Bug Description

There's an inconsistency in TimeRange.always() produced by ZG and libzg. ZG uses (-maxint, maxint), while libzg uses (0, maxint). This introduces incompabilities when using ZG's TimeRange.is_always(), where it'll return True for ZG's TimeRanges and False for libzg's TimeRanges.

Our mightly leader Seif decided that ZG should also use (0, maxint), so this is a reminder to change it for the next release.

Related branches

lp:zeitgeist/0.1
Revision history for this message
Siegfried Gevatter (rainct) wrote : Re: [Bug 614295] [NEW] TimeRange.always() is inconsistent

2010/8/6 Michal Hruby <email address hidden>:
> Our mightly leader Seif decided that ZG should also use (0, maxint), so
> this is a reminder to change it for the next release.

I disagree, if we are using signed integers then "always" includes the
negatives one. (Another thing is whether it really makes sense to have
signed integers or we should switch to unsigned ones, which I think we
should do).

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

I am pretty much indifferent on TimeRange.always() is. The .is_always() function is just used in the fts extension to do a slight optimization in case the time range is irrelevant.

Regarding change of timestamp signature: What's the technical argument for breaking API (and breaking it in a big way)?

Revision history for this message
Siegfried Gevatter (rainct) wrote : Re: [Bug 614295] Re: TimeRange.always() is inconsistent

2010/8/6 Mikkel Kamstrup Erlandsen <email address hidden>:
> Regarding change of timestamp signature: What's the technical argument
> for breaking API (and breaking it in a big way)?

I don't really think we should break it now (just saying that it'd
make more sense, but in practice I don't think it'll make a
difference).

Revision history for this message
Seif Lotfy (seif) wrote :

As far as I can see its not really a breakage of the API. It is under the hood and the interface is still the same.... There are no events prior 1970 stored on the computer. Thus it makes most sense to have something consistent and valid. How do you want to calculate the timestamp for 1901 -.-
(This is just jibberish but I hope by scoping the range down we get a slight performance boost, even if unnoticed)

Revision history for this message
Seif Lotfy (seif) wrote :

We need to settle on this so I can fix it quickly... I hate pesky little bugs

Changed in zeitgeist:
status: New → Triaged
importance: Undecided → Low
importance: Low → Medium
assignee: nobody → Seif Lotfy (seif)
Revision history for this message
Seif Lotfy (seif) wrote :

https://code.edge.launchpad.net/~zeitgeist/zeitgeist/timerange_and_event_value_fix/+merge/32574

Changed in zeitgeist:
status: Triaged → Fix Committed
Siegfried Gevatter (rainct)
Changed in zeitgeist:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.