launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/use-existing into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #258610 Show bzr push command for empty branches
  https://bugs.launchpad.net/bugs/258610


= 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
-- 
https://code.launchpad.net/~abentley/launchpad/use-existing/+merge/43701
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/use-existing into lp:launchpad.

=== 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-14 20:41:15 +0000
@@ -490,6 +490,11 @@
             self.context.stacked_on)
 
     @property
+    def is_empty_directory(self):
+        """True if the branch is an empty directory without even a '.bzr'."""
+        return self.context.control_format is None
+
+    @property
     def codebrowse_url(self):
         """Return the link to codebrowse for this branch."""
         return self.context.codebrowse_url()

=== 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-14 20:41:15 +0000
@@ -37,12 +37,14 @@
     BranchView,
     )
 from lp.code.browser.branchlisting import PersonOwnedBranchesView
+from lp.code.bzr import ControlFormat
 from lp.code.enums import (
     BranchLifecycleStatus,
     BranchType,
     )
 from lp.code.interfaces.branchtarget import IBranchTarget
 from lp.testing import (
+    BrowserTestCase,
     login,
     login_person,
     logout,
@@ -132,7 +134,7 @@
             "<private server>", view.mirror_location)
 
 
-class TestBranchView(TestCaseWithFactory):
+class TestBranchView(BrowserTestCase):
 
     layer = DatabaseFunctionalLayer
 
@@ -248,6 +250,28 @@
         view.initialize()
         self.assertEqual(list(view.translations_sources()), [trunk])
 
+    def test_is_empty_directory(self):
+        # Branches are considered empty until they get a control format.
+        branch = self.factory.makeBranch()
+        view = BranchView(branch, self.request)
+        view.initialize()
+        self.assertTrue(view.is_empty_directory)
+        with person_logged_in(branch.owner):
+            branch.branchChanged(
+                None, None, ControlFormat.BZR_METADIR_1, None, None)
+        self.assertFalse(view.is_empty_directory)
+
+    def test_empty_directories_use_existing(self):
+        # Push example should include --use-existing for empty directories.
+        branch = self.factory.makeBranch(owner=self.user)
+        text = self.getMainText(branch)
+        self.assertIn('push --use-existing', text)
+        with person_logged_in(self.user):
+            branch.branchChanged(
+                None, None, ControlFormat.BZR_METADIR_1, None, None)
+        text = self.getMainText(branch)
+        self.assertNotIn('push --use-existing', text)
+
     def test_user_can_upload(self):
         # A user can upload if they have edit permissions.
         branch = self.factory.makeAnyBranch()

=== 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-14 20:41:15 +0000
@@ -39,7 +39,11 @@
         <tal:can-upload tal:condition="view/user_can_upload">
           <dl id="upload-url">
             <dt>Update this branch:</dt>
-            <dd>bzr push <span class="branch-url" tal:content="context/bzr_identity" /></dd>
+            <dd>bzr push
+              <tal:use-existing condition="view/is_empty_directory">
+              --use-existing
+              </tal:use-existing>
+            <span class="branch-url" tal:content="context/bzr_identity" /></dd>
           </dl>
           <p tal:condition="not:view/user/sshkeys" id="ssh-key-directions">
             To authenticate with the Launchpad branch upload service, you need