loggerhead

Merge lp:~jameinel/loggerhead/authors-733015 into lp:loggerhead

  1. authors-733015
  2. Merge into trunk-rich
Proposed by John A Meinel
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: 437
Merged at revision: 438
Proposed branch: lp:~jameinel/loggerhead/authors-733015
Merge into: lp:loggerhead
Prerequisite: lp:~jameinel/loggerhead/simple_mainline
Diff against target: 261 lines (+131/-23)
6 files modified
NEWS (+6/-3)
loggerhead/history.py (+7/-1)
loggerhead/templates/revisioninfo.pt (+4/-0)
loggerhead/tests/__init__.py (+1/-0)
loggerhead/tests/test_history.py (+78/-19)
loggerhead/tests/test_revision_ui.py (+35/-0)
To merge this branch: bzr merge lp:~jameinel/loggerhead/authors-733015
Reviewer Review Type Date Requested Status
Jelmer Vernooij code Approve
Review via email: mp+53000@code.launchpad.net

Description of the change

This addresses the issue in bug #733015. When a revision has authors separate from the committer, the UI was only showing the authors as the "Committer". This changes the UI so that it separates the two. So it will now show both Author(s) and Committer.

I didn't try to get too fancy with showing/hiding authors when they match the committer. Or handling the plural form exactly. we could add that later.

On the plus side, this is tested. Both at the 'unittest' History produces the correct Container data, but also at the HTML level. (Partly because I don't know another way to test the templating code.)

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

This only depends loosely on 'simple_mainline'. Because simple_mainline added 'test_history.py' and I didn't want to add another one.

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Yay for more testing :)

The change mentioned and the tags fix seem fine to me, though I think hiding the authors if they are the same as the committer should probably be part of this too. Always showing the authors will be a bit of a regression for most revisions. Could you perhaps file a bug about that when this MP lands?

review: Approve (code)
Revision history for this message
John A Meinel (jameinel) wrote :

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

On 3/16/2011 10:53 AM, Jelmer Vernooij wrote:
> Review: Approve code
> Yay for more testing :)
>
> The change mentioned and the tags fix seem fine to me, though I think hiding the authors if they are the same as the committer should probably be part of this too. Always showing the authors will be a bit of a regression for most revisions. Could you perhaps file a bug about that when this MP lands?

The change was pretty easy, so I just did it:
> === modified file 'loggerhead/templates/revisioninfo.pt'
> --- loggerhead/templates/revisioninfo.pt 2011-03-11 10:56:34 +0000
> +++ loggerhead/templates/revisioninfo.pt 2011-03-16 12:37:17 +0000
> @@ -5,7 +5,7 @@
> <strong>Committer:</strong>
> <span tal:content="python:util.hide_email(change.committer)"></span>
> </li>
> - <li class="authors">
> + <li class="authors" tal:condition="python:len(change.authors) > 1 or change.authors[0] != change.committer">
> <strong>Author(s):</strong>
> <span tal:content="python:', '.join(util.hide_emails(change.authors))"></span>
> </li>

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

iEYEARECAAYFAk2ArzwACgkQJdeBCYSNAANwkACeK9is+jQeqeTjk/PGKOoyz6uV
FjoAn3S86blTeP7ejILDV3PVF5RrGaPE
=dlHt
-----END PGP SIGNATURE-----

lp:~jameinel/loggerhead/authors-733015 updated
438. By John A Meinel

Merge trunk and resolve NEWS conflict.

439. By John A Meinel

Merge brown-bag fix for __init__.py

440. By John A Meinel

Suppress the Author(s) field when it matches the committer.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2011-03-04 15:49:43 +0000
3+++ NEWS 2011-03-11 11:07:01 +0000
4@@ -16,12 +16,15 @@
5 - Merge the pqm changes back into trunk, after trunk was reverted to an old
6 revision. (John Arbash Meinel, #716152)
7
8+ - Remove ``start-loggerhead`` and ``stop-loggerhead`` which were already
9+ deprecated. (John Arbash Meinel)
10+
11+ - Show "Author(s)" as separate from "Committer". And label them
12+ correctly. (John Arbash Meinel, #733015)
13+
14 - The json module is no longer claimed to be supported as alternative for
15 simplejson. (Jelmer Vernooij, #586611)
16
17- - Remove ``start-loggerhead`` and ``stop-loggerhead`` which were already
18- deprecated. (John Arbash Meinel)
19-
20
21 1.18 [10Nov2010]
22 ----------------
23
24=== modified file 'loggerhead/history.py'
25--- loggerhead/history.py 2011-03-11 11:07:01 +0000
26+++ loggerhead/history.py 2011-03-11 11:07:01 +0000
27@@ -35,6 +35,7 @@
28 import textwrap
29 import threading
30
31+from bzrlib import tag
32 import bzrlib.branch
33 import bzrlib.delta
34 import bzrlib.errors
35@@ -679,12 +680,17 @@
36
37 revtags = None
38 if revision.revision_id in self._branch_tags:
39- revtags = ', '.join(self._branch_tags[revision.revision_id])
40+ # tag.sort_* functions expect (tag, data) pairs, so we generate them,
41+ # and then strip them
42+ tags = [(t, None) for t in self._branch_tags[revision.revision_id]]
43+ tag.sort_natural(self._branch, tags)
44+ revtags = u', '.join([t[0] for t in tags])
45
46 entry = {
47 'revid': revision.revision_id,
48 'date': datetime.datetime.fromtimestamp(revision.timestamp),
49 'utc_date': datetime.datetime.utcfromtimestamp(revision.timestamp),
50+ 'committer': revision.committer,
51 'authors': revision.get_apparent_authors(),
52 'branch_nick': revision.properties.get('branch-nick', None),
53 'short_comment': short_message,
54
55=== modified file 'loggerhead/templates/revisioninfo.pt'
56--- loggerhead/templates/revisioninfo.pt 2011-02-18 05:41:17 +0000
57+++ loggerhead/templates/revisioninfo.pt 2011-03-11 11:07:01 +0000
58@@ -3,6 +3,10 @@
59 <ul>
60 <li class="committer">
61 <strong>Committer:</strong>
62+ <span tal:content="python:util.hide_email(change.committer)"></span>
63+ </li>
64+ <li class="authors">
65+ <strong>Author(s):</strong>
66 <span tal:content="python:', '.join(util.hide_emails(change.authors))"></span>
67 </li>
68 <li class="timer">
69
70=== modified file 'loggerhead/tests/__init__.py'
71--- loggerhead/tests/__init__.py 2011-03-11 11:07:01 +0000
72+++ loggerhead/tests/__init__.py 2011-03-11 11:07:01 +0000
73@@ -23,6 +23,7 @@
74 'test_history',
75 'test_load_test',
76 'test_simple',
77+ 'test_revision_ui',
78 'test_templating',
79 ]]))
80 return standard_tests
81
82=== modified file 'loggerhead/tests/test_history.py'
83--- loggerhead/tests/test_history.py 2011-03-11 11:07:01 +0000
84+++ loggerhead/tests/test_history.py 2011-03-11 11:07:01 +0000
85@@ -17,9 +17,10 @@
86
87 """Direct tests of the loggerhead/history.py module"""
88
89+from datetime import datetime
90 from bzrlib import tests
91
92-from loggerhead import history
93+from loggerhead import history as _mod_history
94
95
96 class TestHistoryGetRevidsFrom(tests.TestCaseWithMemoryTransport):
97@@ -40,7 +41,7 @@
98 builder.finish_series()
99 b = builder.get_branch()
100 self.addCleanup(b.lock_read().unlock)
101- return history.History(b, {})
102+ return _mod_history.History(b, {})
103
104 def make_merged_ancestry(self):
105 # Time goes up
106@@ -58,7 +59,7 @@
107 builder.finish_series()
108 b = builder.get_branch()
109 self.addCleanup(b.lock_read().unlock)
110- return history.History(b, {})
111+ return _mod_history.History(b, {})
112
113 def make_deep_merged_ancestry(self):
114 # Time goes up
115@@ -83,32 +84,90 @@
116 builder.finish_series()
117 b = builder.get_branch()
118 self.addCleanup(b.lock_read().unlock)
119- return history.History(b, {})
120+ return _mod_history.History(b, {})
121
122- def assertRevidsFrom(self, expected, his, search_revs, tip_rev):
123+ def assertRevidsFrom(self, expected, history, search_revs, tip_rev):
124 self.assertEqual(expected,
125- list(his.get_revids_from(search_revs, tip_rev)))
126+ list(history.get_revids_from(search_revs, tip_rev)))
127
128 def test_get_revids_from_simple_mainline(self):
129- his = self.make_linear_ancestry()
130+ history = self.make_linear_ancestry()
131 self.assertRevidsFrom(['rev-3', 'rev-2', 'rev-1'],
132- his, None, 'rev-3')
133+ history, None, 'rev-3')
134
135 def test_get_revids_from_merged_mainline(self):
136- his = self.make_merged_ancestry()
137+ history = self.make_merged_ancestry()
138 self.assertRevidsFrom(['rev-3', 'rev-1'],
139- his, None, 'rev-3')
140+ history, None, 'rev-3')
141
142 def test_get_revids_given_one_rev(self):
143- his = self.make_merged_ancestry()
144+ history = self.make_merged_ancestry()
145 # rev-3 was the first mainline revision to see rev-2.
146- self.assertRevidsFrom(['rev-3'], his, ['rev-2'], 'rev-3')
147+ self.assertRevidsFrom(['rev-3'], history, ['rev-2'], 'rev-3')
148
149 def test_get_revids_deep_ancestry(self):
150- his = self.make_deep_merged_ancestry()
151- self.assertRevidsFrom(['F'], his, ['F'], 'F')
152- self.assertRevidsFrom(['F'], his, ['E'], 'F')
153- self.assertRevidsFrom(['F'], his, ['D'], 'F')
154- self.assertRevidsFrom(['F'], his, ['C'], 'F')
155- self.assertRevidsFrom(['B'], his, ['B'], 'F')
156- self.assertRevidsFrom(['A'], his, ['A'], 'F')
157+ history = self.make_deep_merged_ancestry()
158+ self.assertRevidsFrom(['F'], history, ['F'], 'F')
159+ self.assertRevidsFrom(['F'], history, ['E'], 'F')
160+ self.assertRevidsFrom(['F'], history, ['D'], 'F')
161+ self.assertRevidsFrom(['F'], history, ['C'], 'F')
162+ self.assertRevidsFrom(['B'], history, ['B'], 'F')
163+ self.assertRevidsFrom(['A'], history, ['A'], 'F')
164+
165+
166+class TestHistoryChangeFromRevision(tests.TestCaseWithTransport):
167+
168+ def make_single_commit(self):
169+ tree = self.make_branch_and_tree('test')
170+ rev_id = tree.commit('Commit Message', timestamp=1299838474.317,
171+ timezone=3600, committer='Joe Example <joe@example.com>',
172+ revprops={})
173+ self.addCleanup(tree.branch.lock_write().unlock)
174+ rev = tree.branch.repository.get_revision(rev_id)
175+ history = _mod_history.History(tree.branch, {})
176+ return history, rev
177+
178+ def test_simple(self):
179+ history, rev = self.make_single_commit()
180+ change = history._change_from_revision(rev)
181+ self.assertEqual(rev.revision_id, change.revid)
182+ self.assertEqual(datetime.fromtimestamp(1299838474.317),
183+ change.date)
184+ self.assertEqual(datetime.utcfromtimestamp(1299838474.317),
185+ change.utc_date)
186+ self.assertEqual(['Joe Example <joe@example.com>'],
187+ change.authors)
188+ self.assertEqual('test', change.branch_nick)
189+ self.assertEqual('Commit Message', change.short_comment)
190+ self.assertEqual('Commit Message', change.comment)
191+ self.assertEqual(['Commit&nbsp;Message'], change.comment_clean)
192+ self.assertEqual([], change.parents)
193+ self.assertEqual([], change.bugs)
194+ self.assertEqual(None, change.tags)
195+
196+ def test_tags(self):
197+ history, rev = self.make_single_commit()
198+ b = history._branch
199+ b.tags.set_tag('tag-1', rev.revision_id)
200+ b.tags.set_tag('tag-2', rev.revision_id)
201+ b.tags.set_tag('Tag-10', rev.revision_id)
202+ change = history._change_from_revision(rev)
203+ # tags are 'naturally' sorted, sorting numbers in order, and ignoring
204+ # case, etc.
205+ self.assertEqual('tag-1, tag-2, Tag-10', change.tags)
206+
207+ def test_committer_vs_authors(self):
208+ tree = self.make_branch_and_tree('test')
209+ rev_id = tree.commit('Commit Message', timestamp=1299838474.317,
210+ timezone=3600, committer='Joe Example <joe@example.com>',
211+ revprops={'authors': u'A Author <aauthor@example.com>\n'
212+ u'B Author <bauthor@example.com>'})
213+ self.addCleanup(tree.branch.lock_write().unlock)
214+ rev = tree.branch.repository.get_revision(rev_id)
215+ history = _mod_history.History(tree.branch, {})
216+ change = history._change_from_revision(rev)
217+ self.assertEqual(u'Joe Example <joe@example.com>',
218+ change.committer)
219+ self.assertEqual([u'A Author <aauthor@example.com>',
220+ u'B Author <bauthor@example.com>'],
221+ change.authors)
222
223=== added file 'loggerhead/tests/test_revision_ui.py'
224--- loggerhead/tests/test_revision_ui.py 1970-01-01 00:00:00 +0000
225+++ loggerhead/tests/test_revision_ui.py 2011-03-11 11:07:01 +0000
226@@ -0,0 +1,35 @@
227+# Copyright (C) 2011 Canonical Ltd.
228+#
229+# This program is free software; you can redistribute it and/or modify
230+# it under the terms of the GNU General Public License as published by
231+# the Free Software Foundation; either version 2 of the License, or
232+# (at your option) any later version.
233+#
234+# This program is distributed in the hope that it will be useful,
235+# but WITHOUT ANY WARRANTY; without even the implied warranty of
236+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
237+# GNU General Public License for more details.
238+#
239+# You should have received a copy of the GNU General Public License
240+# along with this program; if not, write to the Free Software
241+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
242+#
243+
244+
245+from loggerhead.tests.test_simple import BasicTests
246+
247+
248+class TestRevisionUI(BasicTests):
249+
250+ def test_authors_vs_committer(self):
251+ self.createBranch()
252+ self.tree.commit('First', committer="Joe Example <joe@example.com>",
253+ revprops={'authors': u'A Author <aauthor@example.com>\n'
254+ u'B Author <bauthor@example.com>'})
255+ app = self.setUpLoggerhead()
256+ res = app.get('/revision/1')
257+ # We would like to assert that Joe Example is connected to Committer,
258+ # and the Authors are connected. However, that requires asserting the
259+ # exact HTML connections, which I wanted to avoid.
260+ res.mustcontain('Committer', 'Joe Example',
261+ 'Author(s)', 'A Author, B Author')

Subscribers

People subscribed via source and target branches