Merge lp:~mwhudson/loggerhead/bug-321325 into lp:loggerhead

Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: j.c.sackett
Approved revision: 462
Merged at revision: 463
Proposed branch: lp:~mwhudson/loggerhead/bug-321325
Merge into: lp:loggerhead
Diff against target: 53 lines (+8/-8)
2 files modified
loggerhead/history.py (+4/-7)
loggerhead/templates/revisionfilechanges.pt (+4/-1)
To merge this branch: bzr merge lp:~mwhudson/loggerhead/bug-321325
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Richard Harding (community) code* Approve
Review via email: mp+87881@code.launchpad.net

Description of the change

This branch makes two changes in the area of showing exe-bit changes in the revision page:

1) It shows a * by the file name if the exe bit has changed
2) It does not make the name a link if _only_ the exe bit has changed

It doesn't show in which direction the exe bit has changed, because report_changes doesn't tell us.

Kind changes are still not reported at all, but oh well, they can be done if someone actually cares.

To post a comment you must log in.
Revision history for this message
Richard Harding (rharding) wrote :

Michael, why is file_id removed along with the change? I don't see where this report method is called from, so I'm not sure if it's something that isn't needed any more, but it seems that it's not related to the rest of the change to add the * on exe changes.

review: Needs Information (code*)
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Ah sorry, I just removed the file_id as they were no longer used (and were cluttering some debugging I was doing). I should have mentioned that in the cover letter, apologies for that.

Revision history for this message
Richard Harding (rharding) :
review: Approve (code*)
Revision history for this message
j.c.sackett (jcsackett) wrote :

Thanks for clarifying why file_ids are gone. I have nothing else to add.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'loggerhead/history.py'
2--- loggerhead/history.py 2011-11-23 08:29:50 +0000
3+++ loggerhead/history.py 2012-01-08 23:24:25 +0000
4@@ -136,22 +136,19 @@
5 new_revision=self.revid(self.new_inv, file_id)))
6 if versioned == 'added':
7 self.added.append(util.Container(
8- filename=rich_filename(paths[1], kind),
9- file_id=file_id, kind=kind[1]))
10+ filename=rich_filename(paths[1], kind), kind=kind[1]))
11 elif versioned == 'removed':
12 self.removed.append(util.Container(
13- filename=rich_filename(paths[0], kind),
14- file_id=file_id, kind=kind[0]))
15+ filename=rich_filename(paths[0], kind), kind=kind[0]))
16 elif renamed:
17 self.renamed.append(util.Container(
18 old_filename=rich_filename(paths[0], kind[0]),
19 new_filename=rich_filename(paths[1], kind[1]),
20- file_id=file_id,
21- text_modified=modified == 'modified'))
22+ text_modified=modified == 'modified', exe_change=exe_change))
23 else:
24 self.modified.append(util.Container(
25 filename=rich_filename(paths[1], kind),
26- file_id=file_id))
27+ text_modified=modified == 'modified', exe_change=exe_change))
28
29 # The lru_cache is not thread-safe, so we need a lock around it for
30 # all threads.
31
32=== modified file 'loggerhead/templates/revisionfilechanges.pt'
33--- loggerhead/templates/revisionfilechanges.pt 2009-03-19 00:05:41 +0000
34+++ loggerhead/templates/revisionfilechanges.pt 2012-01-08 23:24:25 +0000
35@@ -36,6 +36,7 @@
36 <tal:old-name condition="not:renamed/text_modified" content="renamed/new_filename" class="filename">
37 new_filename
38 </tal:old-name>
39+ <tal:prop-change condition="renamed/exe_change" content="string:*" />
40 </li>
41 </ul>
42
43@@ -44,7 +45,9 @@
44 <li class="desc">files modified:</li>
45 </tal:block>
46 <li class="files">
47- <tal:revision content="structure python:file_link(item.filename)" />
48+ <tal:revision condition="item/text_modified" content="structure python:file_link(item.filename)" />
49+ <tal:revision condition="not:item/text_modified" content="item/filename" />
50+ <tal:prop-change condition="item/exe_change" content="string:*" />
51 </li>
52 </ul>
53 </tal:block>

Subscribers

People subscribed via source and target branches