Launchpad itself

Merge lp:~thumper/launchpad/descriptions-for-merge-proposals into lp:launchpad/db-devel

  1. descriptions-for-merge-proposals
  2. Merge into db-devel
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: not available
Merged at revision: 9035
Proposed branch: lp:~thumper/launchpad/descriptions-for-merge-proposals
Merge into: lp:launchpad/db-devel
Diff against target: 1275 lines (+269/-246)
27 files modified
.bzrignore (+1/-0)
database/schema/patch-2207-33-0.sql (+9/-0)
lib/canonical/launchpad/javascript/code/codereview.js (+49/-34)
lib/lp/code/browser/branch.py (+5/-3)
lib/lp/code/browser/branchmergeproposal.py (+40/-1)
lib/lp/code/browser/configure.zcml (+7/-0)
lib/lp/code/browser/tests/test_branchmergeproposal.py (+9/-20)
lib/lp/code/configure.zcml (+6/-2)
lib/lp/code/doc/branch-merge-proposal-notifications.txt (+1/-1)
lib/lp/code/doc/codereviewcomment.txt (+2/-8)
lib/lp/code/interfaces/branch.py (+15/-18)
lib/lp/code/interfaces/branchmergeproposal.py (+7/-3)
lib/lp/code/mail/branchmergeproposal.py (+12/-19)
lib/lp/code/mail/codehandler.py (+4/-7)
lib/lp/code/mail/tests/test_branchmergeproposal.py (+0/-3)
lib/lp/code/mail/tests/test_codehandler.py (+24/-25)
lib/lp/code/model/branch.py (+4/-7)
lib/lp/code/model/branchmergeproposal.py (+3/-12)
lib/lp/code/model/tests/test_branch.py (+1/-3)
lib/lp/code/model/tests/test_branchmergeproposals.py (+1/-31)
lib/lp/code/model/tests/test_codereviewcomment.py (+0/-3)
lib/lp/code/stories/branches/xx-branchmergeproposals.txt (+1/-1)
lib/lp/code/stories/branches/xx-claiming-team-code-reviews.txt (+2/-1)
lib/lp/code/stories/webservice/xx-branchmergeproposal.txt (+3/-15)
lib/lp/code/templates/branchmergeproposal-index.pt (+53/-24)
lib/lp/code/windmill/tests/test_branchmergeproposal_commitmessage.py (+3/-3)
lib/lp/testing/factory.py (+7/-2)
To merge this branch: bzr merge lp:~thumper/launchpad/descriptions-for-merge-proposals
Reviewer Review Type Date Requested Status
Björn Tillenius (community) db Approve
Stuart Bishop (community) db Approve
Paul Hummer (community) code js Approve
Review via email: mp+19564@code.launchpad.net

Commit message

Add a description to merge proposals. Also removes the initial comment and sets the description instead.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Right now, I just want to check on the size of the diff. Really should be another way to do this :)

Add an editable description to merge proposals, so a little more like bugs.

Revision history for this message
Tim Penhey (thumper) wrote :

Here is a picture with neither the commit message nor the description set:
  http://people.canonical.com/~tim/description-1.png

With a commit message but no description:
  http://people.canonical.com/~tim/description-2.png

With commit message and description set:
http://people.canonical.com/~tim/description-3.png
http://people.canonical.com/~tim/description-4.png

Revision history for this message
Tim Penhey (thumper) wrote :

Damn, hit the wrong key...

Pic 4 contains description but not commit message.

Revision history for this message
Paul Hummer (rockstar) wrote :

206 @property
207 + def description_html(self):
208 + """The description as widget HTML."""
209 + description = self.context.description
210 + if description is None:
211 + description = ''
212 + formatter = FormattersAPI
213 + hide_email = formatter(description).obfuscate_email()
214 + description = formatter(hide_email).text_to_html()
215 + return TextAreaEditorWidget(
216 + self.context,
217 + 'description',
218 + canonical_url(self.context, view_name='+edit-description'),
219 + id="edit-description",
220 + title="Description of the Change",
221 + value=description,
222 + accept_empty=True)
223 +
224 + @property

I don't particularly like this, but it's not an issue that you really need to worry about. I'll bring it up in the next reviewer meeting.

review: Approve (code js)
Revision history for this message
Stuart Bishop (stub) wrote :

Fine if BranchMergeProposal.description really should be NULLable. patch-2207-33-0.sql.

If there will always be a description, I can cook up some data migration in the patch to copy the text from the first message attached to the BranchMergeProposal.

review: Approve (db)
Revision history for this message
Björn Tillenius (bjornt) wrote :

I'm approving this, since I think it's small enough and changes can be done later.

However, I would like to start a discussion on how similar to bugs this should be. If you have comments and description, it would be good to be consistent with other parts of Launchpad. The commenting system in merge proposals are similar enough to bugs, so having them work the same way would be beneficial.

review: Approve (db)
Revision history for this message
Tim Penhey (thumper) wrote :

On Wed, 24 Feb 2010 19:36:27 Björn Tillenius wrote:
> Review: Approve db
> I'm approving this, since I think it's small enough and changes can be done
> later.
>
> However, I would like to start a discussion on how similar to bugs this
> should be. If you have comments and description, it would be good to be
> consistent with other parts of Launchpad. The commenting system in merge
> proposals are similar enough to bugs, so having them work the same way
> would be beneficial.

Ah yes. Francis and I were talking about this earlier. We need to come up
with an architectural solution to remembering history.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '.bzrignore'
2--- .bzrignore 2010-01-22 02:30:19 +0000
3+++ .bzrignore 2010-02-24 08:55:31 +0000
4@@ -58,4 +58,5 @@
5 .subversion
6 lib/canonical/buildd/launchpad-files
7 .testrepository
8+.memcache.pid
9 ./pipes
10
11=== added file 'database/schema/patch-2207-33-0.sql'
12--- database/schema/patch-2207-33-0.sql 1970-01-01 00:00:00 +0000
13+++ database/schema/patch-2207-33-0.sql 2010-02-24 08:55:31 +0000
14@@ -0,0 +1,9 @@
15+-- Copyright 2010 Canonical Ltd. This software is licensed under the
16+-- GNU Affero General Public License version 3 (see the file LICENSE).
17+
18+SET client_min_messages=ERROR;
19+
20+ALTER TABLE BranchMergeProposal
21+ ADD COLUMN description TEXT;
22+
23+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 33, 0);
24
25=== modified file 'lib/canonical/launchpad/javascript/code/codereview.js'
26--- lib/canonical/launchpad/javascript/code/codereview.js 2010-01-29 16:00:43 +0000
27+++ lib/canonical/launchpad/javascript/code/codereview.js 2010-02-24 08:55:31 +0000
28@@ -39,22 +39,9 @@
29 */
30 link.on('click', show_request_review_form);
31 }
32- link = Y.one('.menu-link-set_commit_message');
33- if (Y.Lang.isValue(link)) {
34- link.addClass('js-action');
35- link.on('click', edit_commit_message);
36- }
37- if (Y.Lang.isValue(Y.lp.widgets)) {
38- var widget = Y.lp.widgets['edit-commit-message'];
39- if (Y.Lang.isValue(widget)) {
40- widget.editor.on('save', function() {
41- commit_message_listener(this.get('value'), true);
42- });
43- widget.editor.on('cancel', function() {
44- commit_message_listener(this.get('value'), false);
45- });
46- }
47- }
48+
49+ link_multiline_editor('commit_message');
50+ link_multiline_editor('description');
51 link_scroller('#proposal-summary a.diff-link', '#review-diff');
52 link_scroller('.menu-link-add_comment', '#add-comment', function() {
53 Y.one('#add-comment-form textarea').focus();
54@@ -82,24 +69,54 @@
55 });
56 }
57
58-/*
59- * Hide the commit message editor if the value is empty.
60+
61+/*
62+ * Make the edit link a javascript link (green).
63+ * Link the listener to the save and cancel events of the multiline editor.
64+ */
65+function link_multiline_editor(name) {
66+ var link = Y.one('.menu-link-set_' + name);
67+ if (Y.Lang.isValue(link)) {
68+ link.addClass('js-action');
69+ link.on('click', function(e) {
70+ hide_link_show_multiline_edit(e, name);}
71+ );
72+ var parent = link.ancestor();
73+ if (parent.hasClass('unseen')) {
74+ link.addClass('unseen');
75+ parent.removeClass('unseen');
76+ }
77+ }
78+ if (Y.Lang.isValue(Y.lp.widgets)) {
79+ var widget = Y.lp.widgets['edit-' + name];
80+ if (Y.Lang.isValue(widget)) {
81+ widget.editor.on('save', function() {
82+ multiline_edit_message_listener(name, this.get('value'), true);
83+ });
84+ widget.editor.on('cancel', function() {
85+ multiline_edit_message_listener(name, this.get('value'), false);
86+ });
87+ }
88+ }
89+}
90+
91+
92+/*
93+ * Hide the editor if the value is empty.
94 *
95- * If the commit message is empty, we want to show the
96- * 'Set commit message' link again. For consistency with
97- * page updates we want to flash this link so the user can
98- * see what we are doing. If the commit message was saved
99- * and is empty, then we flash green as all is good. If the
100- * user has cancelled the edit, and the commit message is
101- * empty, then we flash the link red.
102+ * If the value is empty, we want to show the 'Set commit message' link again.
103+ * For consistency with page updates we want to flash this link so the user
104+ * can see what we are doing. If the commit message was saved and is empty,
105+ * then we flash green as all is good. If the user has cancelled the edit,
106+ * and the commit message is empty, then we flash the link red.
107 */
108-function commit_message_listener(message, saved)
109+function multiline_edit_message_listener(name, message, saved)
110 {
111 if (message === '') {
112 // Hide the multiline editor
113- Y.one('#edit-commit-message').addClass('unseen');
114+ Y.one('#edit-' + name).addClass('unseen');
115 // Show the link again
116- var link = Y.one('.menu-link-set_commit_message');
117+ var link = Y.one('.menu-link-set_' + name);
118 link.removeClass('unseen');
119 if (saved) {
120 // Flash green.
121@@ -113,19 +130,17 @@
122 }
123
124 /*
125- * Edit the commit message.
126- *
127 * Hide the link, show the multi-line editor, and set it to edit.
128 */
129-function edit_commit_message(e) {
130+function hide_link_show_multiline_edit(e, name) {
131 // We are handling this click event.
132 e.halt();
133 // Make the edit button unseen.
134- Y.one('#edit-commit-message').removeClass('unseen');
135+ Y.one('#edit-' + name).removeClass('unseen');
136 // Remove the unseen class from the commit message.
137- Y.one('.menu-link-set_commit_message').addClass('unseen');
138+ Y.one('.menu-link-set_' + name).addClass('unseen');
139 // Trigger the edit on the multiline editor.
140- Y.lp.widgets['edit-commit-message']._triggerEdit(e);
141+ Y.lp.widgets['edit-' + name]._triggerEdit(e);
142 }
143
144 /*
145
146=== modified file 'lib/lp/code/browser/branch.py'
147--- lib/lp/code/browser/branch.py 2010-02-23 16:12:55 +0000
148+++ lib/lp/code/browser/branch.py 2010-02-24 08:55:31 +0000
149@@ -1189,8 +1189,10 @@
150 ' will not be shown in the diff.)'))
151
152 comment = Text(
153- title=_('Initial Comment'), required=False,
154- description=_('Describe your change.'))
155+ title=_('Description of the Change'), required=False,
156+ description=_('Describe what changes your branch introduces, '
157+ 'what bugs it fixes, or what features it implements. '
158+ 'Ideally include rationale and how to test.'))
159
160 reviewer = copy_field(
161 ICodeReviewVoteReference['reviewer'], required=False)
162@@ -1260,7 +1262,7 @@
163 registrant=registrant, target_branch=target_branch,
164 prerequisite_branch=prerequisite_branch,
165 needs_review=data['needs_review'],
166- initial_comment=data.get('comment'),
167+ description=data.get('comment'),
168 review_requests=review_requests,
169 commit_message=data.get('commit_message'))
170 self.next_url = canonical_url(proposal)
171
172=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
173--- lib/lp/code/browser/branchmergeproposal.py 2010-02-05 21:59:26 +0000
174+++ lib/lp/code/browser/branchmergeproposal.py 2010-02-24 08:55:31 +0000
175@@ -15,6 +15,7 @@
176 'BranchMergeProposalContextMenu',
177 'BranchMergeProposalDeleteView',
178 'BranchMergeProposalDequeueView',
179+ 'BranchMergeProposalDescriptionEditView',
180 'BranchMergeProposalEditMenu',
181 'BranchMergeProposalEditView',
182 'BranchMergeProposalEnqueueView',
183@@ -197,6 +198,11 @@
184 return Link('+edit', text, icon='edit', enabled=enabled)
185
186 @enabled_with_permission('launchpad.Edit')
187+ def set_description(self):
188+ text = 'Set description'
189+ return Link('+edit-description', text, icon='add')
190+
191+ @enabled_with_permission('launchpad.Edit')
192 def set_commit_message(self):
193 text = 'Set commit message'
194 enabled = self.context.isMergable()
195@@ -295,6 +301,7 @@
196 'add_comment',
197 'dequeue',
198 'set_commit_message',
199+ 'set_description',
200 'edit_status',
201 'enqueue',
202 'merge',
203@@ -662,6 +669,24 @@
204 for bug in self.context.related_bugs]
205
206 @property
207+ def description_html(self):
208+ """The description as widget HTML."""
209+ description = self.context.description
210+ if description is None:
211+ description = ''
212+ formatter = FormattersAPI
213+ hide_email = formatter(description).obfuscate_email()
214+ description = formatter(hide_email).text_to_html()
215+ return TextAreaEditorWidget(
216+ self.context,
217+ 'description',
218+ canonical_url(self.context, view_name='+edit-description'),
219+ id="edit-description",
220+ title="Description of the Change",
221+ value=description,
222+ accept_empty=True)
223+
224+ @property
225 def commit_message_html(self):
226 """The commit message as widget HTML."""
227 commit_message = self.context.commit_message
228@@ -674,7 +699,7 @@
229 self.context,
230 'commit_message',
231 canonical_url(self.context, view_name='+edit-commit-message'),
232- id="edit-commit-message",
233+ id="edit-commit_message",
234 title="Commit Message",
235 value=commit_message,
236 accept_empty=True)
237@@ -977,6 +1002,20 @@
238 self.updateContextFromData(data)
239
240
241+class BranchMergeProposalDescriptionEditView(MergeProposalEditView):
242+ """The view to edit the description of merge proposals."""
243+
244+ schema = IBranchMergeProposal
245+ label = "Edit merge proposal description"
246+ page_title = label
247+ field_names = ['description']
248+
249+ @action('Update', name='update')
250+ def update_action(self, action, data):
251+ """Update the commit message."""
252+ self.updateContextFromData(data)
253+
254+
255 class BranchMergeProposalDeleteView(MergeProposalEditView):
256 """The view to control the deletion of merge proposals."""
257 schema = IBranchMergeProposal
258
259=== modified file 'lib/lp/code/browser/configure.zcml'
260--- lib/lp/code/browser/configure.zcml 2010-02-19 16:33:29 +0000
261+++ lib/lp/code/browser/configure.zcml 2010-02-24 08:55:31 +0000
262@@ -224,6 +224,13 @@
263 permission="launchpad.Edit"
264 template="../../app/templates/generic-edit.pt"/>
265 <browser:page
266+ name="+edit-description"
267+ for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
268+ class="lp.code.browser.branchmergeproposal.BranchMergeProposalDescriptionEditView"
269+ facet="branches"
270+ permission="launchpad.Edit"
271+ template="../../app/templates/generic-edit.pt"/>
272+ <browser:page
273 name="+delete"
274 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
275 class="lp.code.browser.branchmergeproposal.BranchMergeProposalDeleteView"
276
277=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
278--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2010-02-05 21:59:26 +0000
279+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2010-02-24 08:55:31 +0000
280@@ -358,17 +358,6 @@
281 self.assertEqual(self.target_branch, proposal.target_branch)
282 return proposal
283
284- def assertNoComments(self, proposal):
285- # There should be no comments.
286- self.assertEqual([], list(proposal.all_comments))
287-
288- def assertOneComment(self, proposal, comment_text):
289- # There should be one and only one comment with the text specified.
290- self.assertEqual(
291- [comment_text],
292- [comment.message.text_contents
293- for comment in proposal.all_comments])
294-
295 def assertNoPendingReviews(self, proposal):
296 # There should be no votes recorded for the proposal.
297 self.assertEqual([], list(proposal.votes))
298@@ -395,7 +384,7 @@
299 'needs_review': True})
300 proposal = self._getSourceProposal()
301 self.assertNoPendingReviews(proposal)
302- self.assertNoComments(proposal)
303+ self.assertIs(None, proposal.description)
304
305 def test_register_work_in_progress(self):
306 # The needs review checkbox can be unchecked to create a work in
307@@ -420,17 +409,17 @@
308 self.assertEqual('Fixed the bug!', proposal.commit_message)
309
310 def test_register_initial_comment(self):
311- # If the user specifies an initial comment, this is added to the
312+ # If the user specifies a description, this is recorded on the
313 # proposal.
314 view = self._createView()
315 view.register_action.success(
316 {'target_branch': self.target_branch,
317- 'comment': "This is the first comment.",
318+ 'comment': "This is the description.",
319 'needs_review': True})
320
321 proposal = self._getSourceProposal()
322 self.assertNoPendingReviews(proposal)
323- self.assertOneComment(proposal, "This is the first comment.")
324+ self.assertEqual(proposal.description, "This is the description.")
325
326 def test_register_request_reviewer(self):
327 # If the user requests a reviewer, then a pending vote is added to the
328@@ -444,7 +433,7 @@
329
330 proposal = self._getSourceProposal()
331 self.assertOnePendingReview(proposal, reviewer)
332- self.assertNoComments(proposal)
333+ self.assertIs(None, proposal.description)
334
335 def test_register_request_review_type(self):
336 # We can request a specific review type of the reviewer. If we do, it
337@@ -459,10 +448,10 @@
338
339 proposal = self._getSourceProposal()
340 self.assertOnePendingReview(proposal, reviewer, 'god-like')
341- self.assertNoComments(proposal)
342+ self.assertIs(None, proposal.description)
343
344 def test_register_comment_and_review(self):
345- # The user can give an initial comment and request a review from
346+ # The user can give a description and request a review from
347 # someone.
348 reviewer = self.factory.makePerson()
349 view = self._createView()
350@@ -470,12 +459,12 @@
351 {'target_branch': self.target_branch,
352 'reviewer': reviewer,
353 'review_type': 'god-like',
354- 'comment': "This is the first comment.",
355+ 'comment': "This is the description.",
356 'needs_review': True})
357
358 proposal = self._getSourceProposal()
359 self.assertOnePendingReview(proposal, reviewer, 'god-like')
360- self.assertOneComment(proposal, "This is the first comment.")
361+ self.assertEqual(proposal.description, "This is the description.")
362
363
364 class TestBranchMergeProposalView(TestCaseWithFactory):
365
366=== modified file 'lib/lp/code/configure.zcml'
367--- lib/lp/code/configure.zcml 2010-02-22 12:07:03 +0000
368+++ lib/lp/code/configure.zcml 2010-02-24 08:55:31 +0000
369@@ -206,6 +206,7 @@
370 source_branch
371 target_branch
372 prerequisite_branch
373+ description
374 whiteboard
375 queue_status
376 private
377@@ -229,7 +230,6 @@
378 next_preview_diff_job
379 preview_diff
380 votes
381- root_comment
382 all_comments
383 related_bugs
384 isMergable
385@@ -243,7 +243,8 @@
386 <allow interface="lp.code.interfaces.branchtarget.IHasBranchTarget"/>
387 <require
388 permission="launchpad.Edit"
389- set_attributes="whiteboard merged_revno commit_message root_message_id review_diff"
390+ set_attributes="description whiteboard merged_revno commit_message
391+ root_message_id review_diff"
392 attributes="
393 deleteProposal
394 setStatus
395@@ -267,6 +268,9 @@
396 <adapter
397 factory="lp.code.browser.branchmergeproposal.text_xhtml_representation"
398 name="commit_message"/>
399+ <adapter
400+ factory="lp.code.browser.branchmergeproposal.text_xhtml_representation"
401+ name="description"/>
402
403
404 <class class="lp.code.model.branchmergeproposaljob.CreateMergeProposalJob">
405
406=== modified file 'lib/lp/code/doc/branch-merge-proposal-notifications.txt'
407--- lib/lp/code/doc/branch-merge-proposal-notifications.txt 2010-01-05 23:26:44 +0000
408+++ lib/lp/code/doc/branch-merge-proposal-notifications.txt 2010-02-24 08:55:31 +0000
409@@ -153,7 +153,7 @@
410 >>> bmp.deleteProposal()
411 >>> bmp = source_branch.addLandingTarget(
412 ... registrant, target_branch,
413- ... initial_comment=initial_comment, review_requests=reviewers)
414+ ... description=initial_comment, review_requests=reviewers)
415 >>> removeSecurityProxy(bmp).preview_diff = preview_diff
416 >>> [job,] = list(getUtility(IMergeProposalCreatedJobSource).iterReady())
417 >>> job.run(_create_preview=False)
418
419=== modified file 'lib/lp/code/doc/codereviewcomment.txt'
420--- lib/lp/code/doc/codereviewcomment.txt 2009-07-08 18:42:10 +0000
421+++ lib/lp/code/doc/codereviewcomment.txt 2010-02-24 08:55:31 +0000
422@@ -27,14 +27,8 @@
423 >>> comment = merge_proposal.createComment(
424 ... sender, 'Please merge', 'This patch is very nice.')
425
426-The first comment to be created is the root of the code review
427-conversation, and is available as BranchMergeProposal.root_comment.
428-
429- >>> merge_proposal.root_comment == comment
430- True
431-
432-Subsequent comments are marked as replies to the root_comment, by
433-default.
434+The initial email that gets sent out has the message_id stored in the merge
435+proposal. Subsequent comments are marked as replies to the initial email.
436
437 >>> from lp.code.enums import CodeReviewVote
438 >>> comment2 = merge_proposal.createComment(
439
440=== modified file 'lib/lp/code/interfaces/branch.py'
441--- lib/lp/code/interfaces/branch.py 2010-02-18 16:00:24 +0000
442+++ lib/lp/code/interfaces/branch.py 2010-02-24 08:55:31 +0000
443@@ -754,8 +754,8 @@
444 target_branch=Reference(schema=Interface),
445 prerequisite_branch=Reference(schema=Interface),
446 needs_review=Bool(title=_('Needs review'),
447- description=_('If True, set queue_status to NEEDS_REVIEW.'
448- 'Otherwise, it will be WORK_IN_PROGRESS.')),
449+ description=_('If True the proposal needs review.'
450+ 'Otherwise, it will be work in progress.')),
451 initial_comment=Text(
452 title=_('Initial comment'),
453 description=_("Registrant's initial description of proposal.")),
454@@ -774,38 +774,35 @@
455 registrant, target_branch, prerequisite_branch=None,
456 needs_review=True, initial_comment=None, commit_message=None,
457 reviewers=None, review_types=None):
458- """API-oriented version of addLandingTarget.
459-
460- The parameters are the same as addLandingTarget, except that
461- review_requests is split into a list of reviewers and a list of
462- review types.
463+ """Create a new BranchMergeProposal with this branch as the source.
464+
465+ Both the target_branch and the prerequisite_branch, if it is there,
466+ must be branches with the same target as the source branch.
467+
468+ Personal branches (a.k.a. junk branches) cannot specify landing targets.
469 """
470
471 def addLandingTarget(registrant, target_branch, prerequisite_branch=None,
472- whiteboard=None, date_created=None,
473- needs_review=False, initial_comment=None,
474- review_requests=None, review_diff=None,
475- commit_message=None):
476+ date_created=None, needs_review=False,
477+ description=None, review_requests=None,
478+ review_diff=None, commit_message=None):
479 """Create a new BranchMergeProposal with this branch as the source.
480
481 Both the target_branch and the prerequisite_branch, if it is there,
482- must be branches of the same project as the source branch.
483+ must be branches with the same target as the source branch.
484
485- Branches without associated projects, junk branches, cannot
486- specify landing targets.
487+ Personal branches (a.k.a. junk branches) cannot specify landing targets.
488
489 :param registrant: The person who is adding the landing target.
490 :param target_branch: Must be another branch, and different to self.
491 :param prerequisite_branch: Optional but if it is not None, it must be
492 another branch.
493- :param whiteboard: Optional. Just text, notes or instructions
494- pertinant to the landing such as testing notes.
495 :param date_created: Used to specify the date_created value of the
496 merge request.
497 :param needs_review: Used to specify the proposal is ready for
498 review right now.
499- :param initial_comment: An optional initial comment can be added
500- when adding the new target.
501+ :param description: A description of the bugs fixed, features added,
502+ or refactorings.
503 :param review_requests: An optional list of (`Person`, review_type).
504 """
505
506
507=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
508--- lib/lp/code/interfaces/branchmergeproposal.py 2010-01-11 20:07:17 +0000
509+++ lib/lp/code/interfaces/branchmergeproposal.py 2010-02-24 08:55:31 +0000
510@@ -96,6 +96,13 @@
511 description=_(
512 "If True, this proposal is visible only to subscribers.")))
513
514+ description = exported(
515+ Text(title=_('Description of the Change'), required=False,
516+ description=_(
517+ "A detailed description of the changes that are being "
518+ "addressed by the branch being proposed to be merged."),
519+ max_length=50000))
520+
521 whiteboard = Whiteboard(
522 title=_('Whiteboard'), required=False,
523 description=_('Notes about the merge.'))
524@@ -211,9 +218,6 @@
525 date_queued = exported(
526 Datetime(
527 title=_('Date Queued'), required=False, readonly=True))
528- # Cannote use Object as this would cause circular dependencies.
529- root_comment = Attribute(
530- _("The first message in discussion of this merge proposal"))
531 root_message_id = Text(
532 title=_('The email message id from the first message'),
533 required=False)
534
535=== modified file 'lib/lp/code/mail/branchmergeproposal.py'
536--- lib/lp/code/mail/branchmergeproposal.py 2010-02-15 10:12:11 +0000
537+++ lib/lp/code/mail/branchmergeproposal.py 2010-02-24 08:55:31 +0000
538@@ -67,7 +67,7 @@
539
540 def __init__(self, subject, template_name, recipients, merge_proposal,
541 from_address, delta=None, message_id=None,
542- requested_reviews=None, comment=None, preview_diff=None,
543+ requested_reviews=None, preview_diff=None,
544 direct_email=False):
545 BranchMailer.__init__(
546 self, subject, template_name, recipients, from_address, delta,
547@@ -76,7 +76,6 @@
548 if requested_reviews is None:
549 requested_reviews = []
550 self.requested_reviews = requested_reviews
551- self.comment = comment
552 self.preview_diff = preview_diff
553 self.template_params = self._generateTemplateParams()
554 self.direct_email = direct_email
555@@ -106,7 +105,6 @@
556 'branch-merge-proposal-created.txt', recipients, merge_proposal,
557 from_address, message_id=get_msgid(),
558 requested_reviews=merge_proposal.votes,
559- comment=merge_proposal.root_comment,
560 preview_diff=merge_proposal.preview_diff)
561
562 @classmethod
563@@ -140,17 +138,11 @@
564 """Return a mailer for a request to review a BranchMergeProposal."""
565 from_address = cls._format_user_address(from_user)
566 recipients = {reason.subscriber: reason}
567- comment = None
568- if (merge_proposal.root_comment is not None and
569- (merge_proposal.root_comment.message.owner ==
570- merge_proposal.registrant)):
571- comment = merge_proposal.root_comment
572 return cls(
573 '%(proposal_title)s',
574 'review-requested.txt', recipients,
575 merge_proposal, from_address, message_id=get_msgid(),
576- comment=comment, preview_diff=merge_proposal.preview_diff,
577- direct_email=True)
578+ preview_diff=merge_proposal.preview_diff, direct_email=True)
579
580 def _getReplyToAddress(self):
581 """Return the address to use for the reply-to header."""
582@@ -195,13 +187,14 @@
583
584 def _generateTemplateParams(self):
585 """For template params that don't change, calcualte just once."""
586+ proposal = self.merge_proposal
587 params = {
588- 'proposal_registrant': self.merge_proposal.registrant.displayname,
589- 'source_branch': self.merge_proposal.source_branch.bzr_identity,
590- 'target_branch': self.merge_proposal.target_branch.bzr_identity,
591+ 'proposal_registrant': proposal.registrant.displayname,
592+ 'source_branch': proposal.source_branch.bzr_identity,
593+ 'target_branch': proposal.target_branch.bzr_identity,
594 'prerequisite': '',
595- 'proposal_title': self.merge_proposal.title,
596- 'proposal_url': canonical_url(self.merge_proposal),
597+ 'proposal_title': proposal.title,
598+ 'proposal_url': canonical_url(proposal),
599 'edit_subscription': '',
600 'comment': '',
601 'gap': '',
602@@ -210,8 +203,8 @@
603 'diff_cutoff_warning': '',
604 }
605
606- if self.merge_proposal.prerequisite_branch is not None:
607- prereq_url = self.merge_proposal.prerequisite_branch.bzr_identity
608+ if proposal.prerequisite_branch is not None:
609+ prereq_url = proposal.prerequisite_branch.bzr_identity
610 params['prerequisite'] = ' with %s as a prerequisite' % prereq_url
611
612 requested_reviews = []
613@@ -228,8 +221,8 @@
614 params['reviews'] = (''.join(' %s\n' % review
615 for review in requested_reviews))
616
617- if self.comment is not None:
618- params['comment'] = (self.comment.message.text_contents)
619+ if proposal.description is not None:
620+ params['comment'] = (proposal.description)
621 if len(requested_reviews) > 0:
622 params['gap'] = '\n\n'
623
624
625=== modified file 'lib/lp/code/mail/codehandler.py'
626--- lib/lp/code/mail/codehandler.py 2010-02-10 15:04:00 +0000
627+++ lib/lp/code/mail/codehandler.py 2010-02-24 08:55:31 +0000
628@@ -604,13 +604,10 @@
629 target.code_reviewer, submitter, None,
630 _notify_listeners=False)
631
632- if comment_text.strip() == '':
633- comment = None
634- else:
635- comment = bmp.createComment(
636- submitter, message['Subject'], comment_text,
637- _notify_listeners=False)
638- return bmp, comment
639+ comment_text = comment_text.strip()
640+ if comment_text != '':
641+ bmp.description = comment_text
642+ return bmp
643
644 except BranchMergeProposalExists:
645 body = get_error_message(
646
647=== modified file 'lib/lp/code/mail/tests/test_branchmergeproposal.py'
648--- lib/lp/code/mail/tests/test_branchmergeproposal.py 2010-02-15 10:12:11 +0000
649+++ lib/lp/code/mail/tests/test_branchmergeproposal.py 2010-02-24 08:55:31 +0000
650@@ -383,9 +383,6 @@
651 self.assertEqual(
652 'Requester <requester@example.com>', mailer.from_address)
653 self.assertEqual(
654- request.merge_proposal.root_comment,
655- mailer.comment)
656- self.assertEqual(
657 request.merge_proposal.preview_diff,
658 mailer.preview_diff)
659 self.assertRecipientsMatches([request.recipient], mailer)
660
661=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
662--- lib/lp/code/mail/tests/test_codehandler.py 2010-02-10 17:05:36 +0000
663+++ lib/lp/code/mail/tests/test_codehandler.py 2010-02-24 08:55:31 +0000
664@@ -16,6 +16,7 @@
665 from bzrlib.transport import get_transport
666 from bzrlib.urlutils import join as urljoin
667 from bzrlib.workingtree import WorkingTree
668+from storm.store import Store
669 from zope.component import getUtility
670 from zope.interface import directlyProvides, directlyProvidedBy
671 from zope.security.management import setSecurityPolicy
672@@ -469,12 +470,11 @@
673 self.switchDbUser(config.create_merge_proposals.dbuser)
674 code_handler = CodeHandler()
675 pop_notifications()
676- bmp, comment = code_handler.processMergeProposal(message)
677+ bmp = code_handler.processMergeProposal(message)
678 self.assertEqual(source, bmp.source_branch)
679 self.assertEqual(target, bmp.target_branch)
680 self.assertIs(None, bmp.review_diff)
681- self.assertEqual('Hi!\n', comment.message.text_contents)
682- self.assertEqual('My subject', comment.message.subject)
683+ self.assertEqual('Hi!', bmp.description)
684 # No emails are sent.
685 messages = pop_notifications()
686 self.assertEqual(0, len(messages))
687@@ -493,10 +493,10 @@
688 self.factory.makeMergeDirectiveEmail(body=' '))
689 self.switchDbUser(config.create_merge_proposals.dbuser)
690 code_handler = CodeHandler()
691- bmp, comment = code_handler.processMergeProposal(message)
692+ bmp = code_handler.processMergeProposal(message)
693 self.assertEqual(source_branch, bmp.source_branch)
694 self.assertEqual(target_branch, bmp.target_branch)
695- self.assertIs(None, comment)
696+ self.assertIs(None, bmp.description)
697 self.assertEqual(0, bmp.all_comments.count())
698 transaction.commit()
699
700@@ -554,7 +554,7 @@
701 JobRunner.fromReady(CreateMergeProposalJob).runAll()
702 self.assertEqual(target, source.landing_targets[0].target_branch)
703 # Ensure the DB operations violate no constraints.
704- transaction.commit()
705+ Store.of(source).flush()
706
707 def test_processWithUnicodeMergeDirectiveEmail(self):
708 """process creates a comment from a unicode message body."""
709@@ -568,11 +568,10 @@
710 code_handler.process(message, 'merge@code.launchpad.net', file_alias)
711 self.switchDbUser(config.create_merge_proposals.dbuser)
712 JobRunner.fromReady(CreateMergeProposalJob).runAll()
713- comment = source.landing_targets[0].root_comment
714- self.assertIsNot(None, comment)
715- self.assertEqual(u'\u1234', comment.message.text_contents)
716+ proposal = source.landing_targets[0]
717+ self.assertEqual(u'\u1234', proposal.description)
718 # Ensure the DB operations violate no constraints.
719- transaction.commit()
720+ Store.of(proposal).flush()
721
722 def test_processMergeProposalReviewerRequested(self):
723 # The commands in the merge proposal are parsed.
724@@ -586,7 +585,7 @@
725 self.switchDbUser(config.create_merge_proposals.dbuser)
726 code_handler = CodeHandler()
727 pop_notifications()
728- bmp, comment = code_handler.processMergeProposal(message)
729+ bmp = code_handler.processMergeProposal(message)
730 pending_reviews = list(bmp.votes)
731 self.assertEqual(1, len(pending_reviews))
732 self.assertEqual(eric, pending_reviews[0].reviewer)
733@@ -594,7 +593,7 @@
734 messages = pop_notifications()
735 self.assertEqual(0, len(messages))
736 # Ensure the DB operations violate no constraints.
737- transaction.commit()
738+ Store.of(bmp).flush()
739
740 def test_reviewer_with_diff(self):
741 """Requesting a review with a diff works."""
742@@ -625,7 +624,7 @@
743 self.switchDbUser(config.create_merge_proposals.dbuser)
744 code_handler = CodeHandler()
745 pop_notifications()
746- bmp, comment = code_handler.processMergeProposal(message)
747+ bmp = code_handler.processMergeProposal(message)
748 # If no reviewer is specified, then the default reviewer of the target
749 # branch is requested to review.
750 pending_reviews = list(bmp.votes)
751@@ -637,7 +636,7 @@
752 messages = pop_notifications()
753 self.assertEqual(0, len(messages))
754 # Ensure the DB operations violate no constraints.
755- transaction.commit()
756+ Store.of(target_branch).flush()
757
758 def test_processMergeProposalExists(self):
759 """processMergeProposal raises BranchMergeProposalExists
760@@ -649,7 +648,7 @@
761 self.factory.makeMergeDirectiveEmail())
762 self.switchDbUser(config.create_merge_proposals.dbuser)
763 code_handler = CodeHandler()
764- bmp, comment = code_handler.processMergeProposal(message)
765+ bmp = code_handler.processMergeProposal(message)
766 _unused = pop_notifications()
767 transaction.commit()
768 _unused = code_handler.processMergeProposal(message)
769@@ -879,7 +878,7 @@
770 self.useBzrBranches(real_server=True)
771 branch, source, message = self._createTargetSourceAndBundle(
772 format="pack-0.92")
773- bmp, comment = self._processMergeDirective(message)
774+ bmp = self._processMergeDirective(message)
775 self.assertEqual(BranchType.HOSTED, bmp.source_branch.branch_type)
776 self.assertIs(None, bmp.source_branch.next_mirror_time)
777
778@@ -895,7 +894,7 @@
779 # concerned.
780 branch.last_mirrored = None
781 branch.last_mirrored_id = None
782- bmp, comment = self._processMergeDirective(message)
783+ bmp = self._processMergeDirective(message)
784 self.assertEqual(BranchType.REMOTE, bmp.source_branch.branch_type)
785
786 def test_stackable_target(self):
787@@ -906,7 +905,7 @@
788 self.useBzrBranches(real_server=True)
789 branch, source, message = self._createTargetSourceAndBundle(
790 format="1.9")
791- bmp, comment = self._processMergeDirective(message)
792+ bmp = self._processMergeDirective(message)
793 source_bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch)
794 self.assertEqual(BranchType.HOSTED, bmp.source_branch.branch_type)
795 self.assertIsNot(None, bmp.source_branch.next_mirror_time)
796@@ -919,7 +918,7 @@
797 self.useBzrBranches(real_server=True)
798 branch, source, message = self._createTargetSourceAndBundle(
799 format="1.9")
800- bmp, comment = self._processMergeDirective(message)
801+ bmp = self._processMergeDirective(message)
802 # The hosted location should be populated (open succeeds).
803 source_bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch)
804 # Not the mirror (open raises).
805@@ -933,7 +932,7 @@
806 self.useBzrBranches(real_server=True)
807 branch, source, message = self._createTargetSourceAndBundle(
808 format="1.9")
809- bmp, comment = self._processMergeDirective(message)
810+ bmp = self._processMergeDirective(message)
811 # The source branch is stacked on the target.
812 source_bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch)
813 self.assertEqual(
814@@ -955,7 +954,7 @@
815 format="1.9")
816 target_tree = WorkingTree.open('.')
817 target_tree.commit('rev2b')
818- bmp, comment = self._processMergeDirective(message)
819+ bmp = self._processMergeDirective(message)
820 lp_branch = self._openBazaarBranchAsClient(bmp.source_branch)
821 self.assertEqual(source.last_revision(), lp_branch.last_revision())
822
823@@ -999,7 +998,7 @@
824 self.useBzrBranches(real_server=True)
825 lp_source, message = self._createPreexistingSourceAndMessage(
826 target_format="1.9", source_format="1.9", set_stacked=True)
827- bmp, comment = self._processMergeDirective(message)
828+ bmp = self._processMergeDirective(message)
829 # The branch merge proposal should use the existing db branch.
830 self.assertEqual(lp_source, bmp.source_branch)
831 # Now the branch is now scheduled to be mirrorred.
832@@ -1018,7 +1017,7 @@
833 self.useBzrBranches(real_server=True)
834 lp_source, message = self._createPreexistingSourceAndMessage(
835 target_format="1.9", source_format="1.9")
836- bmp, comment = self._processMergeDirective(message)
837+ bmp = self._processMergeDirective(message)
838 # The branch merge proposal should use the existing db branch.
839 self.assertEqual(lp_source, bmp.source_branch)
840 # Now the branch is not scheduled to be mirrorred.
841@@ -1033,7 +1032,7 @@
842 self.useBzrBranches(real_server=True)
843 lp_source, message = self._createPreexistingSourceAndMessage(
844 target_format="pack-0.92", source_format="1.9")
845- bmp, comment = self._processMergeDirective(message)
846+ bmp = self._processMergeDirective(message)
847 # The branch merge proposal should use the existing db branch.
848 self.assertEqual(lp_source, bmp.source_branch)
849 # Now the branch is not scheduled to be mirrorred.
850@@ -1048,7 +1047,7 @@
851 self.useBzrBranches(real_server=True)
852 lp_source, message = self._createPreexistingSourceAndMessage(
853 target_format="1.9", source_format="pack-0.92")
854- bmp, comment = self._processMergeDirective(message)
855+ bmp = self._processMergeDirective(message)
856 # The branch merge proposal should use the existing db branch.
857 self.assertEqual(lp_source, bmp.source_branch)
858 # Now the branch is not scheduled to be mirrorred.
859
860=== modified file 'lib/lp/code/model/branch.py'
861--- lib/lp/code/model/branch.py 2010-02-22 01:44:55 +0000
862+++ lib/lp/code/model/branch.py 2010-02-24 08:55:31 +0000
863@@ -315,7 +315,7 @@
864 def addLandingTarget(self, registrant, target_branch,
865 prerequisite_branch=None, whiteboard=None,
866 date_created=None, needs_review=False,
867- initial_comment=None, review_requests=None,
868+ description=None, review_requests=None,
869 review_diff=None, commit_message=None):
870 """See `IBranch`."""
871 if not self.target.supports_merge_proposals:
872@@ -369,11 +369,8 @@
873 date_created=date_created,
874 date_review_requested=date_review_requested,
875 queue_status=queue_status, review_diff=review_diff,
876- commit_message=commit_message)
877-
878- if initial_comment is not None:
879- bmp.createComment(
880- registrant, None, initial_comment, _notify_listeners=False)
881+ commit_message=commit_message,
882+ description=description)
883
884 for reviewer, review_type in review_requests:
885 bmp.nominateReviewer(
886@@ -397,7 +394,7 @@
887 review_requests = zip(reviewers, review_types)
888 return self.addLandingTarget(
889 registrant, target_branch, prerequisite_branch,
890- needs_review=needs_review, initial_comment=initial_comment,
891+ needs_review=needs_review, description=initial_comment,
892 commit_message=commit_message, review_requests=review_requests)
893
894 def scheduleDiffUpdates(self):
895
896=== modified file 'lib/lp/code/model/branchmergeproposal.py'
897--- lib/lp/code/model/branchmergeproposal.py 2009-12-18 20:14:21 +0000
898+++ lib/lp/code/model/branchmergeproposal.py 2010-02-24 08:55:31 +0000
899@@ -114,6 +114,8 @@
900 prerequisite_branch = ForeignKey(
901 dbName='dependent_branch', foreignKey='Branch', notNull=False)
902
903+ description = StringCol(default=None)
904+
905 whiteboard = StringCol(default=None)
906
907 queue_status = EnumCol(
908@@ -207,17 +209,6 @@
909 """See `IHasBranchTarget`."""
910 return self.source_branch.target
911
912- @property
913- def root_comment(self):
914- return CodeReviewComment.selectOne("""
915- CodeReviewMessage.id in (
916- SELECT CodeReviewMessage.id
917- FROM CodeReviewMessage, Message
918- WHERE CodeReviewMessage.branch_merge_proposal = %d AND
919- CodeReviewMessage.message = Message.id
920- ORDER BY Message.datecreated LIMIT 1)
921- """ % self.id)
922-
923 root_message_id = StringCol(default=None)
924
925 @property
926@@ -517,7 +508,7 @@
927 registrant=registrant,
928 target_branch=self.target_branch,
929 prerequisite_branch=self.prerequisite_branch,
930- whiteboard=self.whiteboard,
931+ description=self.description,
932 needs_review=True, review_requests=review_requests)
933 self.superseded_by = proposal
934 # This sync update is needed to ensure that the transitive
935
936=== modified file 'lib/lp/code/model/tests/test_branch.py'
937--- lib/lp/code/model/tests/test_branch.py 2010-02-02 22:26:04 +0000
938+++ lib/lp/code/model/tests/test_branch.py 2010-02-24 08:55:31 +0000
939@@ -1238,16 +1238,14 @@
940
941 def test_attributeAssignment(self):
942 """Smoke test to make sure the assignments are there."""
943- whiteboard = u"Some whiteboard"
944 commit_message = u'Some commit message'
945 proposal = self.source.addLandingTarget(
946- self.user, self.target, self.prerequisite, whiteboard,
947+ self.user, self.target, self.prerequisite,
948 commit_message=commit_message)
949 self.assertEqual(proposal.registrant, self.user)
950 self.assertEqual(proposal.source_branch, self.source)
951 self.assertEqual(proposal.target_branch, self.target)
952 self.assertEqual(proposal.prerequisite_branch, self.prerequisite)
953- self.assertEqual(proposal.whiteboard, whiteboard)
954 self.assertEqual(proposal.commit_message, commit_message)
955
956 def test__createMergeProposal_with_reviewers(self):
957
958=== modified file 'lib/lp/code/model/tests/test_branchmergeproposals.py'
959--- lib/lp/code/model/tests/test_branchmergeproposals.py 2010-01-07 21:02:00 +0000
960+++ lib/lp/code/model/tests/test_branchmergeproposals.py 2010-02-24 08:55:31 +0000
961@@ -531,36 +531,6 @@
962 "Expected %s, got %s" % (new_queue_order, queue_order))
963
964
965-class TestRootComment(TestCase):
966- """Test the behavior of the root_comment attribute"""
967-
968- layer = DatabaseFunctionalLayer
969-
970- def setUp(self):
971- TestCase.setUp(self)
972- login('foo.bar@canonical.com')
973- self.factory = LaunchpadObjectFactory()
974- self.merge_proposal = self.factory.makeBranchMergeProposal()
975-
976- def test_orderedByDateNotInsertion(self):
977- """Root is determined by create date, not insert order"""
978- counter = time_counter()
979- oldest_date, middle_date, newest_date = [counter.next() for index in
980- (1, 2, 3)]
981- comment1 = self.merge_proposal.createComment(
982- self.merge_proposal.registrant, "Subject",
983- _date_created=middle_date)
984- self.assertEqual(comment1, self.merge_proposal.root_comment)
985- comment2 = self.merge_proposal.createComment(
986- self.merge_proposal.registrant, "Subject",
987- _date_created=newest_date)
988- self.assertEqual(comment1, self.merge_proposal.root_comment)
989- comment3 = self.merge_proposal.createComment(
990- self.merge_proposal.registrant, "Subject",
991- _date_created=oldest_date)
992- self.assertEqual(comment3, self.merge_proposal.root_comment)
993-
994-
995 class TestCreateCommentNotifications(TestCaseWithFactory):
996 """Test the notifications are raised at the right times."""
997
998@@ -1755,7 +1725,7 @@
999 signing_context=signing_context))
1000 job = CreateMergeProposalJob.create(file_alias)
1001 transaction.commit()
1002- proposal, comment = job.run()
1003+ proposal = job.run()
1004 self.assertEqual(proposal.source_branch, source)
1005 self.assertEqual(proposal.target_branch, target)
1006
1007
1008=== modified file 'lib/lp/code/model/tests/test_codereviewcomment.py'
1009--- lib/lp/code/model/tests/test_codereviewcomment.py 2009-10-22 21:17:56 +0000
1010+++ lib/lp/code/model/tests/test_codereviewcomment.py 2010-02-24 08:55:31 +0000
1011@@ -35,7 +35,6 @@
1012 self.assertEqual(None, comment.vote)
1013 self.assertEqual(None, comment.vote_tag)
1014 self.assertEqual(self.submitter, comment.message.owner)
1015- self.assertEqual(comment, self.bmp.root_comment)
1016 self.assertEqual('Message subject', comment.message.subject)
1017 self.assertEqual('Message content', comment.message.chunks[0].content)
1018
1019@@ -45,7 +44,6 @@
1020 self.assertEqual(None, comment.vote)
1021 self.assertEqual(None, comment.vote_tag)
1022 self.assertEqual(self.submitter, comment.message.owner)
1023- self.assertEqual(comment, self.bmp.root_comment)
1024 self.assertEqual(
1025 'Re: [Merge] %s into %s' % (
1026 self.bmp.source_branch.bzr_identity,
1027@@ -58,7 +56,6 @@
1028 reply = self.bmp.createComment(
1029 self.reviewer, 'Reply subject', 'Reply content',
1030 CodeReviewVote.ABSTAIN, 'My tag', comment)
1031- self.assertEqual(comment, self.bmp.root_comment)
1032 self.assertEqual(comment.message.id, reply.message.parent.id)
1033 self.assertEqual(comment.message, reply.message.parent)
1034 self.assertEqual('Reply subject', reply.message.subject)
1035
1036=== modified file 'lib/lp/code/stories/branches/xx-branchmergeproposals.txt'
1037--- lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2010-02-18 17:31:54 +0000
1038+++ lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2010-02-24 08:55:31 +0000
1039@@ -100,7 +100,7 @@
1040 ... 'Add more <b>mojo</b>')
1041 >>> nopriv_browser.getControl('Update').click()
1042
1043- >>> print_tag_with_id(nopriv_browser.contents, 'edit-commit-message')
1044+ >>> print_tag_with_id(nopriv_browser.contents, 'edit-commit_message')
1045 Commit Message
1046 Add more &lt;b&gt;mojo&lt;/b&gt;
1047
1048
1049=== modified file 'lib/lp/code/stories/branches/xx-claiming-team-code-reviews.txt'
1050--- lib/lp/code/stories/branches/xx-claiming-team-code-reviews.txt 2009-08-07 02:56:17 +0000
1051+++ lib/lp/code/stories/branches/xx-claiming-team-code-reviews.txt 2010-02-24 08:55:31 +0000
1052@@ -21,7 +21,8 @@
1053 >>> eric_browser = setupBrowser(auth="Basic eric@example.com:test")
1054 >>> eric_browser.open(branch_url)
1055 >>> eric_browser.getLink('Propose for merging').click()
1056- >>> eric_browser.getControl('Initial Comment').value = 'Initial comment'
1057+ >>> eric_browser.getControl('Description of the Change').value = (
1058+ ... 'This fix is awesome!')
1059 >>> eric_browser.getControl('Propose Merge').click()
1060
1061 The reviewer table shows both the Vikings team and Eric himself.
1062
1063=== modified file 'lib/lp/code/stories/webservice/xx-branchmergeproposal.txt'
1064--- lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2010-02-04 16:52:05 +0000
1065+++ lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2010-02-24 08:55:31 +0000
1066@@ -35,6 +35,7 @@
1067 date_queued: None
1068 date_review_requested: u'...'
1069 date_reviewed: None
1070+ description: u'Merge\nit!'
1071 merge_reporter_link: None
1072 merged_revno: None
1073 prerequisite_branch_link: u'http://api.launchpad.dev/beta/~...'
1074@@ -57,20 +58,6 @@
1075 votes_collection_link:
1076 u'http://api.launchpad.dev/beta/~.../+merge/.../votes'
1077
1078- >>> all_comments = webservice.get(
1079- ... bmp['all_comments_collection_link']).jsonBody()
1080- >>> pprint_entry(all_comments['entries'][0])
1081- as_quoted_email: u'> Merge\n> it!'
1082- branch_merge_proposal_link:
1083- u'http://api.launchpad.dev/beta/~person-name.../product-name.../branch.../+merge/...'
1084- id: ...
1085- message_body: u'Merge\nit!'
1086- resource_type_link: u'http://api.launchpad.dev/beta/#code_review_comment'
1087- self_link: u'http://api.launchpad.dev/beta/~person-name.../product-name.../branch.../+merge/.../comments/...'
1088- title: u'Comment on proposed merge of lp://dev/~person-name.../product-name.../branch... into lp://dev/~person-name.../product-name.../branch...'
1089- vote: None
1090- vote_tag: None
1091-
1092 Our review request is listed in the votes collection.
1093
1094 >>> votes = webservice.get(
1095@@ -129,6 +116,7 @@
1096 date_queued: None
1097 date_review_requested: None
1098 date_reviewed: None
1099+ description: None
1100 merge_reporter_link: None
1101 merged_revno: None
1102 prerequisite_branch_link: None
1103@@ -170,7 +158,7 @@
1104 vote_tag: u'code'
1105
1106 >>> comment_2 = webservice.named_get(
1107- ... merge_proposal['self_link'], 'getComment', id=3).jsonBody()
1108+ ... merge_proposal['self_link'], 'getComment', id=2).jsonBody()
1109 >>> pprint_entry(comment_2)
1110 as_quoted_email: u'> This is mediocre work.'
1111 branch_merge_proposal_link: u'http://.../~source/fooix/fix-it/+merge/...'
1112
1113=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
1114--- lib/lp/code/templates/branchmergeproposal-index.pt 2010-01-15 04:34:31 +0000
1115+++ lib/lp/code/templates/branchmergeproposal-index.pt 2010-02-24 08:55:31 +0000
1116@@ -18,8 +18,8 @@
1117 #code-review-votes {
1118 margin: 1em 0;
1119 }
1120- #commit-message, #edit-commit-message {
1121- margin: 1em 0 0 0;
1122+ #description, #edit-description {
1123+ margin: 1em 0;
1124 }
1125 #add-comment-form {
1126 max-width: 60em;
1127@@ -39,8 +39,16 @@
1128 #add-comment-review-fields div {
1129 display: inline;
1130 }
1131+ #proposal-summary th {
1132+ font-weight: bold;
1133+ color: #717171;
1134+ }
1135+ #proposal-summary td {
1136+ padding-left: 0.5em;
1137+ }
1138 /* A page-specific fix for inline text are editing to line up box. */
1139- #edit-commit-message .yui-ieditor-input { top: 0; }
1140+ #edit-description .yui-ieditor-input { top: 0; }
1141+ #edit-commit_message .yui-ieditor-input { top: 0; }
1142 </style>
1143 </metal:block>
1144
1145@@ -66,27 +74,6 @@
1146 <tal:summary replace="structure context/@@+pagelet-summary" />
1147 </div>
1148
1149- <div id="commit-message" class="yui-g">
1150- <tal:no-commit-message condition="not: context/commit_message">
1151- <div tal:define="link menu/set_commit_message"
1152- tal:condition="link/enabled"
1153- tal:content="structure link/render">
1154- Set commit message
1155- </div>
1156- <div id="edit-commit-message" class="lazr-multiline-edit unseen"
1157- tal:content="structure view/commit_message_html"/>
1158- </tal:no-commit-message>
1159- <tal:has-commit-message condition="context/commit_message">
1160- <div tal:define="link menu/set_commit_message"
1161- tal:condition="link/enabled"
1162- tal:content="structure link/render" class="unseen">
1163- Set commit message
1164- </div>
1165- <div id="edit-commit-message" class="lazr-multiline-edit"
1166- tal:content="structure view/commit_message_html"/>
1167- </tal:has-commit-message>
1168- </div>
1169-
1170 <div class="yui-g">
1171 <div id="votes-target"
1172 tal:content="structure context/@@+votes" />
1173@@ -109,6 +96,48 @@
1174 </p>
1175 </div>
1176
1177+ <div id="commit-message" class="yui-g">
1178+ <tal:no-commit-message condition="not: context/commit_message">
1179+ <div tal:define="link menu/set_commit_message"
1180+ tal:condition="link/enabled"
1181+ tal:content="structure link/render">
1182+ Set commit message
1183+ </div>
1184+ <div id="edit-commit_message" class="lazr-multiline-edit unseen"
1185+ tal:content="structure view/commit_message_html"/>
1186+ </tal:no-commit-message>
1187+ <tal:has-commit-message condition="context/commit_message">
1188+ <div tal:define="link menu/set_commit_message"
1189+ tal:condition="link/enabled"
1190+ tal:content="structure link/render" class="unseen">
1191+ Set commit message
1192+ </div>
1193+ <div id="edit-commit_message" class="lazr-multiline-edit"
1194+ tal:content="structure view/commit_message_html"/>
1195+ </tal:has-commit-message>
1196+ </div>
1197+
1198+ <div id="description" class="yui-g">
1199+ <tal:no-description condition="not: context/description">
1200+ <div tal:define="link menu/set_description"
1201+ tal:condition="link/enabled"
1202+ tal:content="structure link/render">
1203+ Set description
1204+ </div>
1205+ <div id="edit-description" class="lazr-multiline-edit unseen"
1206+ tal:content="structure view/description_html"/>
1207+ </tal:no-description>
1208+ <tal:has-description condition="context/description">
1209+ <div tal:define="link menu/set_description"
1210+ tal:condition="link/enabled"
1211+ tal:content="structure link/render" class="unseen">
1212+ Set description
1213+ </div>
1214+ <div id="edit-description" class="lazr-multiline-edit"
1215+ tal:content="structure view/description_html"/>
1216+ </tal:has-description>
1217+ </div>
1218+
1219 <div class="yui-g">
1220 <tal:not-logged-in condition="not: view/user">
1221 <div align="center" id="add-comment-login-first">
1222
1223=== modified file 'lib/lp/code/windmill/tests/test_branchmergeproposal_commitmessage.py'
1224--- lib/lp/code/windmill/tests/test_branchmergeproposal_commitmessage.py 2010-02-01 18:37:00 +0000
1225+++ lib/lp/code/windmill/tests/test_branchmergeproposal_commitmessage.py 2010-02-24 08:55:31 +0000
1226@@ -21,12 +21,12 @@
1227 # There seem to be two textareas rendered for the yui-ieditor-input for some
1228 # reason.
1229 EDIT_COMMENT_TEXTBOX = (
1230- u'//div[@id="edit-commit-message"]//textarea[@class="yui-ieditor-input"][1]')
1231+ u'//div[@id="edit-commit_message"]//textarea[@class="yui-ieditor-input"][1]')
1232 EDIT_COMMENT_SUBMIT = (
1233- u'//div[@id="edit-commit-message"]//'
1234+ u'//div[@id="edit-commit_message"]//'
1235 'button[contains(@class, "yui-ieditor-submit_button")]')
1236 COMMIT_MESSAGE_TEXT = (
1237- u'//div[@id="edit-commit-message"]//div[@class="yui-editable_text-text"]')
1238+ u'//div[@id="edit-commit_message"]//div[@class="yui-editable_text-text"]')
1239
1240
1241 class TestCommitMessage(WindmillTestCase):
1242
1243=== modified file 'lib/lp/testing/factory.py'
1244--- lib/lp/testing/factory.py 2010-02-23 16:12:55 +0000
1245+++ lib/lp/testing/factory.py 2010-02-24 08:55:31 +0000
1246@@ -917,7 +917,8 @@
1247 set_state=None, prerequisite_branch=None,
1248 product=None, review_diff=None,
1249 initial_comment=None, source_branch=None,
1250- preview_diff=None, date_created=None):
1251+ preview_diff=None, date_created=None,
1252+ description=None):
1253 """Create a proposal to merge based on anonymous branches."""
1254 if target_branch is not None:
1255 target = target_branch.target
1256@@ -932,6 +933,10 @@
1257 target_branch = self.makeProductBranch(product)
1258 target = target_branch.target
1259
1260+ # Fall back to initial_comment for description.
1261+ if description is None:
1262+ description = initial_comment
1263+
1264 if target_branch is None:
1265 target_branch = self.makeBranchTargetBranch(target)
1266 if source_branch is None:
1267@@ -941,7 +946,7 @@
1268 proposal = source_branch.addLandingTarget(
1269 registrant, target_branch,
1270 prerequisite_branch=prerequisite_branch, review_diff=review_diff,
1271- initial_comment=initial_comment, date_created=date_created)
1272+ description=description, date_created=date_created)
1273
1274 unsafe_proposal = removeSecurityProxy(proposal)
1275 if preview_diff is not None:

Subscribers

People subscribed via source and target branches

to status/vote changes: