Launchpad itself

Merge lp:~abentley/launchpad/use-existing into lp:launchpad

  1. use-existing
  2. Merge into devel
Proposed by Aaron Bentley
Status: Merged
Approved by: Māris Fogels
Approved revision: no longer in the source branch.
Merged at revision: 12077
Proposed branch: lp:~abentley/launchpad/use-existing
Merge into: lp:launchpad
Diff against target: 114 lines (+40/-4)
4 files modified
lib/lp/code/browser/branch.py (+5/-0)
lib/lp/code/browser/tests/test_branch.py (+27/-1)
lib/lp/code/stories/branches/xx-upload-directions.txt (+3/-2)
lib/lp/code/templates/branch-management.pt (+5/-1)
To merge this branch: bzr merge lp:~abentley/launchpad/use-existing
Reviewer Review Type Date Requested Status
Māris Fogels (community) Approve
Review via email: mp+43701@code.launchpad.net

Commit message

[r=mars][ui=none][bug=258610] Suggest --use-existing for empty branches.

Description of the change

= Summary =
Fix bug #258610: Show bzr push command for empty branches

== Proposed fix ==
When a branch is completely empty, without even a .bzr directory, show "push
--use-existing $BRANCH" instead of "push $BRANCH".

== Pre-implementation notes ==
None

== Implementation details ==
When the initial push is performed, branchChanged will be called immediately
(before the branch scan), and this will update the control format. So as soon
as the initial push is performed, the branch will not be detected as an empty
directory.

== Tests ==
bin/test -t test_empty_directories_use_existing -t test_is_empty_directory code.browser

Test time: 1.921

== Demo and Q/A ==
Create a hosted branch through the web UI by selecting "Register a branch".
It should show --use-existing in the push command.
Push to the branch. Now it should not show --use-existing.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/browser/branch.py
  lib/lp/code/browser/tests/test_branch.py
  lib/lp/code/templates/branch-management.pt

To post a comment you must log in.
Revision history for this message
Māris Fogels (mars) wrote :

Hi Aaron,

This changes looks good. Nice tests. r=mars

The only recommendation I have is somehow making it clear that setting the ControlFormat(?) counts as creating an existing .bzr directory. The command is black magic to me (perhaps it's obvious to someone who works in the Bazaar domain? Something like .add_bzr_control_directory()? :)

Maris

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2010-12-01 11:26:57 +0000
+++ lib/lp/code/browser/branch.py 2010-12-15 15:06:45 +0000
@@ -490,6 +490,11 @@
490 self.context.stacked_on)490 self.context.stacked_on)
491491
492 @property492 @property
493 def is_empty_directory(self):
494 """True if the branch is an empty directory without even a '.bzr'."""
495 return self.context.control_format is None
496
497 @property
493 def codebrowse_url(self):498 def codebrowse_url(self):
494 """Return the link to codebrowse for this branch."""499 """Return the link to codebrowse for this branch."""
495 return self.context.codebrowse_url()500 return self.context.codebrowse_url()
496501
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2010-12-09 07:14:34 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2010-12-15 15:06:45 +0000
@@ -37,12 +37,14 @@
37 BranchView,37 BranchView,
38 )38 )
39from lp.code.browser.branchlisting import PersonOwnedBranchesView39from lp.code.browser.branchlisting import PersonOwnedBranchesView
40from lp.code.bzr import ControlFormat
40from lp.code.enums import (41from lp.code.enums import (
41 BranchLifecycleStatus,42 BranchLifecycleStatus,
42 BranchType,43 BranchType,
43 )44 )
44from lp.code.interfaces.branchtarget import IBranchTarget45from lp.code.interfaces.branchtarget import IBranchTarget
45from lp.testing import (46from lp.testing import (
47 BrowserTestCase,
46 login,48 login,
47 login_person,49 login_person,
48 logout,50 logout,
@@ -132,7 +134,7 @@
132 "<private server>", view.mirror_location)134 "<private server>", view.mirror_location)
133135
134136
135class TestBranchView(TestCaseWithFactory):137class TestBranchView(BrowserTestCase):
136138
137 layer = DatabaseFunctionalLayer139 layer = DatabaseFunctionalLayer
138140
@@ -248,6 +250,30 @@
248 view.initialize()250 view.initialize()
249 self.assertEqual(list(view.translations_sources()), [trunk])251 self.assertEqual(list(view.translations_sources()), [trunk])
250252
253 def test_is_empty_directory(self):
254 # Branches are considered empty until they get a control format.
255 branch = self.factory.makeBranch()
256 view = BranchView(branch, self.request)
257 view.initialize()
258 self.assertTrue(view.is_empty_directory)
259 with person_logged_in(branch.owner):
260 # Make it look as though the branch has been pushed.
261 branch.branchChanged(
262 None, None, ControlFormat.BZR_METADIR_1, None, None)
263 self.assertFalse(view.is_empty_directory)
264
265 def test_empty_directories_use_existing(self):
266 # Push example should include --use-existing for empty directories.
267 branch = self.factory.makeBranch(owner=self.user)
268 text = self.getMainText(branch)
269 self.assertIn('push\n--use-existing', text)
270 with person_logged_in(self.user):
271 # Make it look as though the branch has been pushed.
272 branch.branchChanged(
273 None, None, ControlFormat.BZR_METADIR_1, None, None)
274 text = self.getMainText(branch)
275 self.assertNotIn('push\n--use-existing', text)
276
251 def test_user_can_upload(self):277 def test_user_can_upload(self):
252 # A user can upload if they have edit permissions.278 # A user can upload if they have edit permissions.
253 branch = self.factory.makeAnyBranch()279 branch = self.factory.makeAnyBranch()
254280
=== modified file 'lib/lp/code/stories/branches/xx-upload-directions.txt'
--- lib/lp/code/stories/branches/xx-upload-directions.txt 2010-08-16 11:36:08 +0000
+++ lib/lp/code/stories/branches/xx-upload-directions.txt 2010-12-15 15:06:45 +0000
@@ -70,7 +70,7 @@
70 >>> instructions = find_tag_by_id(content, 'upload-directions')70 >>> instructions = find_tag_by_id(content, 'upload-directions')
71 >>> print extract_text(instructions)71 >>> print extract_text(instructions)
72 Update this branch:72 Update this branch:
73 bzr push lp://dev/~name12/gnome-terminal/pushed73 bzr push --use-existing lp://dev/~name12/gnome-terminal/pushed
7474
7575
76== SSH key directions ==76== SSH key directions ==
@@ -178,7 +178,8 @@
178 >>> instructions = find_tag_by_id(content, 'upload-directions')178 >>> instructions = find_tag_by_id(content, 'upload-directions')
179 >>> print extract_text(instructions)179 >>> print extract_text(instructions)
180 Update this branch:180 Update this branch:
181 bzr push lp://dev/~landscape-developers/gnome-terminal/pushed181 bzr push --use-existing
182 lp://dev/~landscape-developers/gnome-terminal/pushed
182183
183184
184== Import branch ==185== Import branch ==
185186
=== modified file 'lib/lp/code/templates/branch-management.pt'
--- lib/lp/code/templates/branch-management.pt 2010-06-10 07:54:59 +0000
+++ lib/lp/code/templates/branch-management.pt 2010-12-15 15:06:45 +0000
@@ -39,7 +39,11 @@
39 <tal:can-upload tal:condition="view/user_can_upload">39 <tal:can-upload tal:condition="view/user_can_upload">
40 <dl id="upload-url">40 <dl id="upload-url">
41 <dt>Update this branch:</dt>41 <dt>Update this branch:</dt>
42 <dd>bzr push <span class="branch-url" tal:content="context/bzr_identity" /></dd>42 <dd>bzr push
43 <tal:use-existing condition="view/is_empty_directory">
44 --use-existing
45 </tal:use-existing>
46 <span class="branch-url" tal:content="context/bzr_identity" /></dd>
43 </dl>47 </dl>
44 <p tal:condition="not:view/user/sshkeys" id="ssh-key-directions">48 <p tal:condition="not:view/user/sshkeys" id="ssh-key-directions">
45 To authenticate with the Launchpad branch upload service, you need49 To authenticate with the Launchpad branch upload service, you need