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

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
1=== modified file 'lib/lp/code/browser/branch.py'
2--- lib/lp/code/browser/branch.py 2010-12-01 11:26:57 +0000
3+++ lib/lp/code/browser/branch.py 2010-12-15 15:06:45 +0000
4@@ -490,6 +490,11 @@
5 self.context.stacked_on)
6
7 @property
8+ def is_empty_directory(self):
9+ """True if the branch is an empty directory without even a '.bzr'."""
10+ return self.context.control_format is None
11+
12+ @property
13 def codebrowse_url(self):
14 """Return the link to codebrowse for this branch."""
15 return self.context.codebrowse_url()
16
17=== modified file 'lib/lp/code/browser/tests/test_branch.py'
18--- lib/lp/code/browser/tests/test_branch.py 2010-12-09 07:14:34 +0000
19+++ lib/lp/code/browser/tests/test_branch.py 2010-12-15 15:06:45 +0000
20@@ -37,12 +37,14 @@
21 BranchView,
22 )
23 from lp.code.browser.branchlisting import PersonOwnedBranchesView
24+from lp.code.bzr import ControlFormat
25 from lp.code.enums import (
26 BranchLifecycleStatus,
27 BranchType,
28 )
29 from lp.code.interfaces.branchtarget import IBranchTarget
30 from lp.testing import (
31+ BrowserTestCase,
32 login,
33 login_person,
34 logout,
35@@ -132,7 +134,7 @@
36 "<private server>", view.mirror_location)
37
38
39-class TestBranchView(TestCaseWithFactory):
40+class TestBranchView(BrowserTestCase):
41
42 layer = DatabaseFunctionalLayer
43
44@@ -248,6 +250,30 @@
45 view.initialize()
46 self.assertEqual(list(view.translations_sources()), [trunk])
47
48+ def test_is_empty_directory(self):
49+ # Branches are considered empty until they get a control format.
50+ branch = self.factory.makeBranch()
51+ view = BranchView(branch, self.request)
52+ view.initialize()
53+ self.assertTrue(view.is_empty_directory)
54+ with person_logged_in(branch.owner):
55+ # Make it look as though the branch has been pushed.
56+ branch.branchChanged(
57+ None, None, ControlFormat.BZR_METADIR_1, None, None)
58+ self.assertFalse(view.is_empty_directory)
59+
60+ def test_empty_directories_use_existing(self):
61+ # Push example should include --use-existing for empty directories.
62+ branch = self.factory.makeBranch(owner=self.user)
63+ text = self.getMainText(branch)
64+ self.assertIn('push\n--use-existing', text)
65+ with person_logged_in(self.user):
66+ # Make it look as though the branch has been pushed.
67+ branch.branchChanged(
68+ None, None, ControlFormat.BZR_METADIR_1, None, None)
69+ text = self.getMainText(branch)
70+ self.assertNotIn('push\n--use-existing', text)
71+
72 def test_user_can_upload(self):
73 # A user can upload if they have edit permissions.
74 branch = self.factory.makeAnyBranch()
75
76=== modified file 'lib/lp/code/stories/branches/xx-upload-directions.txt'
77--- lib/lp/code/stories/branches/xx-upload-directions.txt 2010-08-16 11:36:08 +0000
78+++ lib/lp/code/stories/branches/xx-upload-directions.txt 2010-12-15 15:06:45 +0000
79@@ -70,7 +70,7 @@
80 >>> instructions = find_tag_by_id(content, 'upload-directions')
81 >>> print extract_text(instructions)
82 Update this branch:
83- bzr push lp://dev/~name12/gnome-terminal/pushed
84+ bzr push --use-existing lp://dev/~name12/gnome-terminal/pushed
85
86
87 == SSH key directions ==
88@@ -178,7 +178,8 @@
89 >>> instructions = find_tag_by_id(content, 'upload-directions')
90 >>> print extract_text(instructions)
91 Update this branch:
92- bzr push lp://dev/~landscape-developers/gnome-terminal/pushed
93+ bzr push --use-existing
94+ lp://dev/~landscape-developers/gnome-terminal/pushed
95
96
97 == Import branch ==
98
99=== modified file 'lib/lp/code/templates/branch-management.pt'
100--- lib/lp/code/templates/branch-management.pt 2010-06-10 07:54:59 +0000
101+++ lib/lp/code/templates/branch-management.pt 2010-12-15 15:06:45 +0000
102@@ -39,7 +39,11 @@
103 <tal:can-upload tal:condition="view/user_can_upload">
104 <dl id="upload-url">
105 <dt>Update this branch:</dt>
106- <dd>bzr push <span class="branch-url" tal:content="context/bzr_identity" /></dd>
107+ <dd>bzr push
108+ <tal:use-existing condition="view/is_empty_directory">
109+ --use-existing
110+ </tal:use-existing>
111+ <span class="branch-url" tal:content="context/bzr_identity" /></dd>
112 </dl>
113 <p tal:condition="not:view/user/sshkeys" id="ssh-key-directions">
114 To authenticate with the Launchpad branch upload service, you need