Merge lp:~wgrant/launchpad/git-fixes into lp:launchpad

Proposed by William Grant
Status: Merged
Merged at revision: 17474
Proposed branch: lp:~wgrant/launchpad/git-fixes
Merge into: lp:launchpad
Diff against target: 103 lines (+21/-20)
4 files modified
lib/lp/code/interfaces/branch.py (+4/-3)
lib/lp/code/model/branch.py (+3/-1)
lib/lp/code/model/codereviewcomment.py (+2/-2)
lib/lp/code/model/diff.py (+12/-14)
To merge this branch: bzr merge lp:~wgrant/launchpad/git-fixes
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+257977@code.launchpad.net

Commit message

Fix Git merge proposal diff JSON rendering, Unicode diffs, and comment rendering.

Description of the change

Fix a few trivial issues with Git MPs:

 - Fix Git PreviewDiff JSON rendering.

 - Fix non-ASCII git diffs.

 - Fix codereviewcomment rendering OOPS for git MPs.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/interfaces/branch.py'
2--- lib/lp/code/interfaces/branch.py 2015-04-22 16:41:41 +0000
3+++ lib/lp/code/interfaces/branch.py 2015-04-30 22:13:56 +0000
4@@ -358,11 +358,12 @@
5 description=_("Unique name of the branch, including the "
6 "owner and project names.")))
7
8- displayname = exported(
9+ display_name = exported(
10 Text(title=_('Display name'), readonly=True,
11 description=_(
12- "The branch unique_name.")),
13- exported_as='display_name')
14+ "The branch unique_name.")))
15+
16+ displayname = Attribute("Display name (deprecated)")
17
18 code_reviewer = Attribute(
19 "The reviewer if set, otherwise the owner of the branch.")
20
21=== modified file 'lib/lp/code/model/branch.py'
22--- lib/lp/code/model/branch.py 2015-04-22 16:41:41 +0000
23+++ lib/lp/code/model/branch.py 2015-04-30 22:13:56 +0000
24@@ -675,10 +675,12 @@
25 return safe_open('lp-internal', self.getInternalBzrUrl())
26
27 @property
28- def displayname(self):
29+ def display_name(self):
30 """See `IBranch`."""
31 return self.bzr_identity
32
33+ displayname = display_name
34+
35 @property
36 def code_reviewer(self):
37 """See `IBranch`."""
38
39=== modified file 'lib/lp/code/model/codereviewcomment.py'
40--- lib/lp/code/model/codereviewcomment.py 2015-04-15 13:41:13 +0000
41+++ lib/lp/code/model/codereviewcomment.py 2015-04-30 22:13:56 +0000
42@@ -95,8 +95,8 @@
43 @property
44 def title(self):
45 return ('Comment on proposed merge of %(source)s into %(target)s' %
46- {'source': self.branch_merge_proposal.source_branch.displayname,
47- 'target': self.branch_merge_proposal.target_branch.displayname,
48+ {'source': self.branch_merge_proposal.merge_source.display_name,
49+ 'target': self.branch_merge_proposal.merge_source.display_name,
50 })
51
52 @property
53
54=== modified file 'lib/lp/code/model/diff.py'
55--- lib/lp/code/model/diff.py 2015-04-30 19:11:18 +0000
56+++ lib/lp/code/model/diff.py 2015-04-30 22:13:56 +0000
57@@ -12,6 +12,7 @@
58
59 from contextlib import nested
60 from cStringIO import StringIO
61+from operator import attrgetter
62 import sys
63 from uuid import uuid1
64
65@@ -471,8 +472,9 @@
66 conflicts = u"".join(
67 u"Conflict in %s\n" % path for path in response['conflicts'])
68 preview = cls.create(
69- bmp, response['patch'], source_revision_id, target_revision_id,
70- prerequisite_revision_id, conflicts, strip_prefix_segments=1)
71+ bmp, response['patch'].encode('utf-8'), source_revision_id,
72+ target_revision_id, prerequisite_revision_id, conflicts,
73+ strip_prefix_segments=1)
74 del get_property_cache(bmp).preview_diffs
75 del get_property_cache(bmp).preview_diff
76 return preview
77@@ -513,18 +515,14 @@
78 # A preview diff is stale if the revision ids used to make the diff
79 # are different from the tips of the source or target branches.
80 bmp = self.branch_merge_proposal
81- if (self.source_revision_id != bmp.source_branch.last_scanned_id or
82- self.target_revision_id != bmp.target_branch.last_scanned_id):
83- # This is the simple frequent case.
84- return True
85-
86- # More complex involves the prerequisite branch too.
87- if (bmp.prerequisite_branch is not None and
88- (self.prerequisite_revision_id !=
89- bmp.prerequisite_branch.last_scanned_id)):
90- return True
91- else:
92- return False
93+ get_id = attrgetter(
94+ 'last_scanned_id' if bmp.source_branch else 'commit_sha1')
95+ if (self.source_revision_id != get_id(bmp.merge_source) or
96+ self.target_revision_id != get_id(bmp.merge_target)):
97+ return True
98+ return (
99+ bmp.merge_prerequisite is not None and
100+ self.prerequisite_revision_id != get_id(bmp.merge_prerequisite))
101
102 def getFileByName(self, filename):
103 """See `IPreviewDiff`."""