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
=== modified file 'NEWS'
--- NEWS 2012-02-02 04:58:46 +0000
+++ NEWS 2012-02-02 07:48:19 +0000
@@ -1,8 +1,8 @@
1What's changed in loggerhead?1What's changed in loggerhead?
2=============================2=============================
33
41.19 [????]4NEXT
5-----------5----
66
7 - Add ``bzr load-test-loggerhead`` as a way to make sure loggerhead can7 - Add ``bzr load-test-loggerhead`` as a way to make sure loggerhead can
8 handle concurrent requests, etc. Scripts can be written that spawn8 handle concurrent requests, etc. Scripts can be written that spawn
@@ -52,6 +52,9 @@
52 - Add a script and documentation for running under mod_wsgi.52 - Add a script and documentation for running under mod_wsgi.
53 (Stuart Colville, Toshio Kuratomi)53 (Stuart Colville, Toshio Kuratomi)
5454
55 - Make tz calculations consistent and use UTC in the UI everywhere we show
56 a precise timestamp. (Robert Collins, #594591)
57
5558
561.18.1 [24Mar2011]591.18.1 [24Mar2011]
57------------------60------------------
5861
=== modified file 'loggerhead/controllers/directory_ui.py'
--- loggerhead/controllers/directory_ui.py 2009-10-17 06:35:33 +0000
+++ loggerhead/controllers/directory_ui.py 2012-02-02 07:48:19 +0000
@@ -35,7 +35,7 @@
35 if branch is not None:35 if branch is not None:
36 # If a branch is empty, bzr raises an exception when trying this36 # If a branch is empty, bzr raises an exception when trying this
37 try:37 try:
38 self.last_change = datetime.datetime.fromtimestamp(38 self.last_change = datetime.datetime.utcfromtimestamp(
39 branch.repository.get_revision(39 branch.repository.get_revision(
40 branch.last_revision()).timestamp)40 branch.last_revision()).timestamp)
41 except:41 except:
4242
=== modified file 'loggerhead/templates/changelog.pt'
--- loggerhead/templates/changelog.pt 2011-02-23 03:01:15 +0000
+++ loggerhead/templates/changelog.pt 2012-02-02 07:48:19 +0000
@@ -129,8 +129,8 @@
129 <td tal:condition="show_tag_col" tal:content="string:${entry/tags}"129 <td tal:condition="show_tag_col" tal:content="string:${entry/tags}"
130 class="tagcell"></td>130 class="tagcell"></td>
131 <td class="date">131 <td class="date">
132 <span tal:attributes="title python:util.date_time(entry.date)"132 <span tal:attributes="title python:util.date_time(entry.utc_date)"
133 tal:content="python:util._approximatedate(entry.date)"></span>133 tal:content="python:util._approximatedate(entry.utc_date)"></span>
134 </td>134 </td>
135 <td class="diffr"><a tal:attributes="title python:'Show diff at revision '+entry.revno;135 <td class="diffr"><a tal:attributes="title python:'Show diff at revision '+entry.revno;
136 href python:url(['/revision', entry.revno], clear=1)">136 href python:url(['/revision', entry.revno], clear=1)">
137137
=== modified file 'loggerhead/templates/inventory.pt'
--- loggerhead/templates/inventory.pt 2011-04-22 18:18:02 +0000
+++ loggerhead/templates/inventory.pt 2012-02-02 07:48:19 +0000
@@ -84,7 +84,7 @@
84 title string:Show revision ${file/change/revno}"84 title string:Show revision ${file/change/revno}"
85 tal:content="file/change/revno"></a>85 tal:content="file/change/revno"></a>
86 </td>86 </td>
87 <td class="date" tal:content="python:util.date_time(file.change.date)"></td>87 <td class="date" tal:content="python:util.date_time(file.change.utc_date)"></td>
88 <td class="timedate2"></td>88 <td class="timedate2"></td>
89 <td class="expcell"><a tal:attributes="href python:url(['/changes'], start_revid=start_revid, filter_file_id=file.file_id);89 <td class="expcell"><a tal:attributes="href python:url(['/changes'], start_revid=start_revid, filter_file_id=file.file_id);
90 title string:Show revision ${file/change/revno}">90 title string:Show revision ${file/change/revno}">
@@ -107,7 +107,7 @@
107 title string:Show revision ${file/change/revno}"107 title string:Show revision ${file/change/revno}"
108 tal:content="file/change/revno"></a>108 tal:content="file/change/revno"></a>
109 </td>109 </td>
110 <td class="date" tal:content="python:util.date_time(file.change.date)"></td>110 <td class="date" tal:content="python:util.date_time(file.change.utc_date)"></td>
111 <td class="timedate2">.</td>111 <td class="timedate2">.</td>
112 <td class="expcell"><a tal:attributes="href python:url(['/changes'], start_revid=start_revid, filter_file_id=file.file_id);112 <td class="expcell"><a tal:attributes="href python:url(['/changes'], start_revid=start_revid, filter_file_id=file.file_id);
113 title string:Show revision ${file/change/revno}">113 title string:Show revision ${file/change/revno}">
@@ -136,7 +136,7 @@
136 title string:Show revision ${file/change/revno}"136 title string:Show revision ${file/change/revno}"
137 tal:content="file/change/revno"></a>137 tal:content="file/change/revno"></a>
138 </td>138 </td>
139 <td class="date" tal:content="python:util.date_time(file.change.date)"></td>139 <td class="date" tal:content="python:util.date_time(file.change.utc_date)"></td>
140 <td class="timedate2" tal:content="python:util.human_size(file.size)"></td>140 <td class="timedate2" tal:content="python:util.human_size(file.size)"></td>
141 <td class="expcell"><a tal:attributes="href python:url(['/view', revno_url, file.absolutepath]);141 <td class="expcell"><a tal:attributes="href python:url(['/view', revno_url, file.absolutepath]);
142 title string:View ${file/filename}">142 title string:View ${file/filename}">
143143
=== modified file 'loggerhead/templates/revisioninfo.pt'
--- loggerhead/templates/revisioninfo.pt 2011-03-16 12:37:17 +0000
+++ loggerhead/templates/revisioninfo.pt 2012-02-02 07:48:19 +0000
@@ -11,7 +11,7 @@
11 </li>11 </li>
12 <li class="timer">12 <li class="timer">
13 <strong>Date:</strong>13 <strong>Date:</strong>
14 <span tal:content="python:util.date_time(change.date)"></span>14 <span tal:content="python:util.date_time(change.utc_date)"></span>
15 </li>15 </li>
16 <li class="mfrom" tal:condition="python:change.parents[1:]">16 <li class="mfrom" tal:condition="python:change.parents[1:]">
17 <strong>mfrom:</strong>17 <strong>mfrom:</strong>
1818
=== modified file 'loggerhead/templates/view.pt'
--- loggerhead/templates/view.pt 2011-04-22 17:22:26 +0000
+++ loggerhead/templates/view.pt 2012-02-02 07:48:19 +0000
@@ -79,7 +79,7 @@
79 class="viewRev">79 class="viewRev">
80 <a tal:content="python:'%s' % (anno.change.revno,)"80 <a tal:content="python:'%s' % (anno.change.revno,)"
81 tal:attributes="href python:url(['/revision', anno.change.revno], clear=1);81 tal:attributes="href python:url(['/revision', anno.change.revno], clear=1);
82 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>82 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>
83 <span class="viewAuthors" tal:content="python:'by ' + ', '.join(util.hide_emails(anno.change.authors))" />83 <span class="viewAuthors" tal:content="python:'by ' + ', '.join(util.hide_emails(anno.change.authors))" />
84 <br />84 <br />
8585
8686
=== modified file 'loggerhead/util.py'
--- loggerhead/util.py 2012-01-08 21:48:52 +0000
+++ loggerhead/util.py 2012-02-02 07:48:19 +0000
@@ -52,13 +52,12 @@
52# Display of times.52# Display of times.
5353
54# date_day -- just the day54# date_day -- just the day
55# date_time -- full date with time55# date_time -- full date with time (UTC)
56#56#
57# displaydate -- for use in sentences
58# approximatedate -- for use in tables57# approximatedate -- for use in tables
59#58#
60# displaydate and approximatedate return an elementtree <span> Element59# approximatedate return an elementtree <span> Element
61# with the full date in a tooltip.60# with the full date (UTC) in a tooltip.
6261
6362
64def date_day(value):63def date_day(value):
@@ -67,19 +66,12 @@
6766
68def date_time(value):67def date_time(value):
69 if value is not None:68 if value is not None:
70 return value.strftime('%Y-%m-%d %H:%M:%S')69 # Note: this assumes that the value is UTC in some fashion.
70 return value.strftime('%Y-%m-%d %H:%M:%S UTC')
71 else:71 else:
72 return 'N/A'72 return 'N/A'
7373
7474
75def _displaydate(date):
76 delta = abs(datetime.datetime.now() - date)
77 if delta > datetime.timedelta(1, 0, 0):
78 # far in the past or future, display the date
79 return 'on ' + date_day(date)
80 return _approximatedate(date)
81
82
83def _approximatedate(date):75def _approximatedate(date):
84 delta = datetime.datetime.now() - date76 delta = datetime.datetime.now() - date
85 if abs(delta) > datetime.timedelta(1, 0, 0):77 if abs(delta) > datetime.timedelta(1, 0, 0):
@@ -126,10 +118,6 @@
126 return _wrap_with_date_time_title(date, _approximatedate(date))118 return _wrap_with_date_time_title(date, _approximatedate(date))
127119
128120
129def displaydate(date):
130 return _wrap_with_date_time_title(date, _displaydate(date))
131
132
133class Container(object):121class Container(object):
134 """122 """
135 Convert a dict into an object with attributes.123 Convert a dict into an object with attributes.

Subscribers

People subscribed via source and target branches