loggerhead

Merge lp:~lifeless/loggerhead/bug-594591 into lp:loggerhead

  1. bug-594591
  2. Merge into trunk-rich
Proposed by Robert Collins
Status: Merged
Approved by: John A Meinel
Approved revision: 466
Merged at revision: 466
Proposed branch: lp:~lifeless/loggerhead/bug-594591
Merge into: lp:loggerhead
Diff against target: 163 lines (+18/-27)
7 files modified
NEWS (+5/-2)
loggerhead/controllers/directory_ui.py (+1/-1)
loggerhead/templates/changelog.pt (+2/-2)
loggerhead/templates/inventory.pt (+3/-3)
loggerhead/templates/revisioninfo.pt (+1/-1)
loggerhead/templates/view.pt (+1/-1)
loggerhead/util.py (+5/-17)
To merge this branch: bzr merge lp:~lifeless/loggerhead/bug-594591
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+91212@code.launchpad.net

Description of the change

Its a bit fugly as history.py doesn't utilise timezones (but perhaps all bzr timestamps are natively in utc? I don't know?. Anyhow, this looks correct, and if its wrong, only two places (history.py and the directory last_changed calculation will need fixing (and its no -more- wrong than what we have today).

Also deleted some unused code.

No tests (because none broke :(). Thats an existing tech-debt we need to fix, but I don't think it makes this change less valuable.

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/2/2012 8:51 AM, Robert Collins wrote:
> The proposal to merge lp:~lifeless/loggerhead/bug-594591 into
> lp:loggerhead has been updated.
>
> Description changed to:
>
> Its a bit fugly as history.py doesn't utilise timezones (but
> perhaps all bzr timestamps are natively in utc? I don't know?.
> Anyhow, this looks correct, and if its wrong, only two places
> (history.py and the directory last_changed calculation will need
> fixing (and its no -more- wrong than what we have today).
>

Internally bzr timestamps are time-since-epoch, which AIUI means they
are effectively UTC, and we store a timezone to be able to translate
the timestamp into the local time of the committer for things like
'bzr log'.

> Also deleted some unused code.
>
> No tests (because none broke :(). Thats an existing tech-debt we
> need to fix, but I don't think it makes this change less valuable.
>
> For more details, see:
> https://code.launchpad.net/~lifeless/loggerhead/bug-594591/+merge/91212

As
>
 for whether it is wrong... It depends on whether you want to
display the UTC time for every committer, or whether you want to
display their local time.

- From the bug report, it appears that Loggerhead was reporting the
times as server-local time, which is probably the worst of all
options. So +1 for being UTC, +2 probably if we changed it to be the
local-time of the committer. (But really, then we'd probably need an
option, etc.)

 merge: approve

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk8qQpcACgkQJdeBCYSNAANdawCgojGZQpTk2qmcFmM6SsgufTx9
RhMAn0Vnm0G6W72WHC0DzG/zTU3ayszo
=zcCS
-----END PGP SIGNATURE-----

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2012-02-02 04:58:46 +0000
3+++ NEWS 2012-02-02 07:48:19 +0000
4@@ -1,8 +1,8 @@
5 What's changed in loggerhead?
6 =============================
7
8-1.19 [????]
9------------
10+NEXT
11+----
12
13 - Add ``bzr load-test-loggerhead`` as a way to make sure loggerhead can
14 handle concurrent requests, etc. Scripts can be written that spawn
15@@ -52,6 +52,9 @@
16 - Add a script and documentation for running under mod_wsgi.
17 (Stuart Colville, Toshio Kuratomi)
18
19+ - Make tz calculations consistent and use UTC in the UI everywhere we show
20+ a precise timestamp. (Robert Collins, #594591)
21+
22
23 1.18.1 [24Mar2011]
24 ------------------
25
26=== modified file 'loggerhead/controllers/directory_ui.py'
27--- loggerhead/controllers/directory_ui.py 2009-10-17 06:35:33 +0000
28+++ loggerhead/controllers/directory_ui.py 2012-02-02 07:48:19 +0000
29@@ -35,7 +35,7 @@
30 if branch is not None:
31 # If a branch is empty, bzr raises an exception when trying this
32 try:
33- self.last_change = datetime.datetime.fromtimestamp(
34+ self.last_change = datetime.datetime.utcfromtimestamp(
35 branch.repository.get_revision(
36 branch.last_revision()).timestamp)
37 except:
38
39=== modified file 'loggerhead/templates/changelog.pt'
40--- loggerhead/templates/changelog.pt 2011-02-23 03:01:15 +0000
41+++ loggerhead/templates/changelog.pt 2012-02-02 07:48:19 +0000
42@@ -129,8 +129,8 @@
43 <td tal:condition="show_tag_col" tal:content="string:${entry/tags}"
44 class="tagcell"></td>
45 <td class="date">
46- <span tal:attributes="title python:util.date_time(entry.date)"
47- tal:content="python:util._approximatedate(entry.date)"></span>
48+ <span tal:attributes="title python:util.date_time(entry.utc_date)"
49+ tal:content="python:util._approximatedate(entry.utc_date)"></span>
50 </td>
51 <td class="diffr"><a tal:attributes="title python:'Show diff at revision '+entry.revno;
52 href python:url(['/revision', entry.revno], clear=1)">
53
54=== modified file 'loggerhead/templates/inventory.pt'
55--- loggerhead/templates/inventory.pt 2011-04-22 18:18:02 +0000
56+++ loggerhead/templates/inventory.pt 2012-02-02 07:48:19 +0000
57@@ -84,7 +84,7 @@
58 title string:Show revision ${file/change/revno}"
59 tal:content="file/change/revno"></a>
60 </td>
61- <td class="date" tal:content="python:util.date_time(file.change.date)"></td>
62+ <td class="date" tal:content="python:util.date_time(file.change.utc_date)"></td>
63 <td class="timedate2"></td>
64 <td class="expcell"><a tal:attributes="href python:url(['/changes'], start_revid=start_revid, filter_file_id=file.file_id);
65 title string:Show revision ${file/change/revno}">
66@@ -107,7 +107,7 @@
67 title string:Show revision ${file/change/revno}"
68 tal:content="file/change/revno"></a>
69 </td>
70- <td class="date" tal:content="python:util.date_time(file.change.date)"></td>
71+ <td class="date" tal:content="python:util.date_time(file.change.utc_date)"></td>
72 <td class="timedate2">.</td>
73 <td class="expcell"><a tal:attributes="href python:url(['/changes'], start_revid=start_revid, filter_file_id=file.file_id);
74 title string:Show revision ${file/change/revno}">
75@@ -136,7 +136,7 @@
76 title string:Show revision ${file/change/revno}"
77 tal:content="file/change/revno"></a>
78 </td>
79- <td class="date" tal:content="python:util.date_time(file.change.date)"></td>
80+ <td class="date" tal:content="python:util.date_time(file.change.utc_date)"></td>
81 <td class="timedate2" tal:content="python:util.human_size(file.size)"></td>
82 <td class="expcell"><a tal:attributes="href python:url(['/view', revno_url, file.absolutepath]);
83 title string:View ${file/filename}">
84
85=== modified file 'loggerhead/templates/revisioninfo.pt'
86--- loggerhead/templates/revisioninfo.pt 2011-03-16 12:37:17 +0000
87+++ loggerhead/templates/revisioninfo.pt 2012-02-02 07:48:19 +0000
88@@ -11,7 +11,7 @@
89 </li>
90 <li class="timer">
91 <strong>Date:</strong>
92- <span tal:content="python:util.date_time(change.date)"></span>
93+ <span tal:content="python:util.date_time(change.utc_date)"></span>
94 </li>
95 <li class="mfrom" tal:condition="python:change.parents[1:]">
96 <strong>mfrom:</strong>
97
98=== modified file 'loggerhead/templates/view.pt'
99--- loggerhead/templates/view.pt 2011-04-22 17:22:26 +0000
100+++ loggerhead/templates/view.pt 2012-02-02 07:48:19 +0000
101@@ -79,7 +79,7 @@
102 class="viewRev">
103 <a tal:content="python:'%s' % (anno.change.revno,)"
104 tal:attributes="href python:url(['/revision', anno.change.revno], clear=1);
105- title python:'%s by %s, on %s (%s)' % (anno.change.revno, ', '.join(util.hide_emails(anno.change.authors)), anno.change.date.strftime('%d %b %Y %H:%M'), util.date_time(anno.change.date))"></a>
106+ title python:'%s by %s, on %s (%s)' % (anno.change.revno, ', '.join(util.hide_emails(anno.change.authors)), anno.change.utc_date.strftime('%d %b %Y %H:%M UTC'), util.date_time(anno.change.utc_date))"></a>
107 <span class="viewAuthors" tal:content="python:'by ' + ', '.join(util.hide_emails(anno.change.authors))" />
108 <br />
109
110
111=== modified file 'loggerhead/util.py'
112--- loggerhead/util.py 2012-01-08 21:48:52 +0000
113+++ loggerhead/util.py 2012-02-02 07:48:19 +0000
114@@ -52,13 +52,12 @@
115 # Display of times.
116
117 # date_day -- just the day
118-# date_time -- full date with time
119+# date_time -- full date with time (UTC)
120 #
121-# displaydate -- for use in sentences
122 # approximatedate -- for use in tables
123 #
124-# displaydate and approximatedate return an elementtree <span> Element
125-# with the full date in a tooltip.
126+# approximatedate return an elementtree <span> Element
127+# with the full date (UTC) in a tooltip.
128
129
130 def date_day(value):
131@@ -67,19 +66,12 @@
132
133 def date_time(value):
134 if value is not None:
135- return value.strftime('%Y-%m-%d %H:%M:%S')
136+ # Note: this assumes that the value is UTC in some fashion.
137+ return value.strftime('%Y-%m-%d %H:%M:%S UTC')
138 else:
139 return 'N/A'
140
141
142-def _displaydate(date):
143- delta = abs(datetime.datetime.now() - date)
144- if delta > datetime.timedelta(1, 0, 0):
145- # far in the past or future, display the date
146- return 'on ' + date_day(date)
147- return _approximatedate(date)
148-
149-
150 def _approximatedate(date):
151 delta = datetime.datetime.now() - date
152 if abs(delta) > datetime.timedelta(1, 0, 0):
153@@ -126,10 +118,6 @@
154 return _wrap_with_date_time_title(date, _approximatedate(date))
155
156
157-def displaydate(date):
158- return _wrap_with_date_time_title(date, _displaydate(date))
159-
160-
161 class Container(object):
162 """
163 Convert a dict into an object with attributes.

Subscribers

People subscribed via source and target branches