launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/requested-sync-grammar-bug-826752-these into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #826752 in Launchpad itself: ""Requested sync of 1 packages." is bad grammar"
  https://bugs.launchpad.net/launchpad/+bug/826752

For more details, see:
https://code.launchpad.net/~allenap/launchpad/requested-sync-grammar-bug-826752-these/+merge/71878

The first fix for this bug only adjusted the first part of the
message. I completely failed to notice the incorrect messages that
could be generated:

  Requested sync of 0 packages.
  Please allow some time for these to be processed.
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  (i.e. processing 0 packages won't take very long.)

and:

  Requested sync of 1 package.
  Please allow some time for these to be processed.
                             ^^^^^
  (i.e. should be "this".)

This branch fixes the first by not showing the "Please allow ..." line
at all, and uses "this" instead of "these" in the second case.

-- 
https://code.launchpad.net/~allenap/launchpad/requested-sync-grammar-bug-826752-these/+merge/71878
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/requested-sync-grammar-bug-826752-these into lp:launchpad.

=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py	2011-08-16 19:34:28 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py	2011-08-17 12:54:43 +0000
@@ -2049,7 +2049,7 @@
         notifications = view.request.response.notifications
         self.assertEqual(1, len(notifications))
         self.assertIn(
-            "<p>Requested sync of 1 package.</p>",
+            "Requested sync of 1 package.",
             notifications[0].message)
         # 302 is a redirect back to the same page.
         self.assertEqual(302, view.request.response.getStatus())
@@ -2271,20 +2271,19 @@
 
     def test_zero_packages(self):
         self.assertEqual(
-            "<p>Requested sync of 0 packages.</p><p>Please allow "
-            "some time for these to be processed.</p>",
+            "Requested sync of 0 packages.",
             copy_asynchronously_message(0).escapedtext)
 
     def test_one_package(self):
         self.assertEqual(
-            "<p>Requested sync of 1 package.</p><p>Please allow "
-            "some time for these to be processed.</p>",
+            "Requested sync of 1 package.<br />Please "
+            "allow some time for this to be processed.",
             copy_asynchronously_message(1).escapedtext)
 
     def test_multiple_packages(self):
         self.assertEqual(
-            "<p>Requested sync of 5 packages.</p><p>Please allow "
-            "some time for these to be processed.</p>",
+            "Requested sync of 5 packages.<br />Please "
+            "allow some time for these to be processed.",
             copy_asynchronously_message(5).escapedtext)
 
 

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2011-08-16 11:32:52 +0000
+++ lib/lp/soyuz/browser/archive.py	2011-08-17 12:54:43 +0000
@@ -1300,10 +1300,17 @@
     """
     package_or_packages = get_plural_text(
         source_pubs_count, "package", "packages")
-    return structured(
-        "<p>Requested sync of %s %s.</p>"
-        "<p>Please allow some time for these to be processed.</p>",
-        source_pubs_count, package_or_packages)
+    this_or_these = get_plural_text(
+        source_pubs_count, "this", "these")
+    if source_pubs_count == 0:
+        return structured(
+            "Requested sync of %s %s.",
+            source_pubs_count, package_or_packages)
+    else:
+        return structured(
+            "Requested sync of %s %s.<br />"
+            "Please allow some time for %s to be processed.",
+            source_pubs_count, package_or_packages, this_or_these)
 
 
 def render_cannotcopy_as_html(cannotcopy_exception):