launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04043
[Merge] lp:~jml/launchpad/xxx-cleanup into lp:launchpad
Jonathan Lange has proposed merging lp:~jml/launchpad/xxx-cleanup into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jml/launchpad/xxx-cleanup/+merge/65663
This branch cleans up a few of my XXX comments from over the years.
* lib/lp/soyuz/browser/sourcepackagerelease.py had a few generic Launchpad text parsing functions that are now in lp.app.browser.stringformatting
* canonical.launchpad.ftests.test_tales is dead
* Wrap a syscall in tachandler with until_no_eintr, so that it's robust
* Put an __all__ in c.l.ftests to avoid pyflakes lint
* Removed a XXX comment that was relevant only when the bug was open
* Got rid of a duplicate call to get_authors()
* Passed through 'line' in our warninghandler, now that we aren't on Python 2.5
Happy to answer questions. Sorry for the grab bag approach, but it seemed easier for all concerned. Mostly me though.
--
https://code.launchpad.net/~jml/launchpad/xxx-cleanup/+merge/65663
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jml/launchpad/xxx-cleanup into lp:launchpad.
=== modified file 'lib/canonical/launchpad/daemons/tachandler.py'
--- lib/canonical/launchpad/daemons/tachandler.py 2011-06-21 16:47:51 +0000
+++ lib/canonical/launchpad/daemons/tachandler.py 2011-06-23 13:20:14 +0000
@@ -25,6 +25,7 @@
kill_by_pidfile,
remove_if_exists,
two_stage_kill,
+ until_no_eintr,
)
@@ -85,9 +86,7 @@
proc = subprocess.Popen(args, stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
self.addCleanup(self.killTac)
- # XXX: JonathanLange 2008-03-19: This can raise EINTR. We should
- # really catch it and try again if that happens.
- stdout = proc.stdout.read()
+ stdout = until_no_eintr(10, proc.stdout.read)
if stdout:
raise TacException('Error running %s: unclean stdout/err: %s'
% (args, stdout))
=== modified file 'lib/canonical/launchpad/doc/badges.txt'
--- lib/canonical/launchpad/doc/badges.txt 2010-10-31 20:18:45 +0000
+++ lib/canonical/launchpad/doc/badges.txt 2011-06-23 13:20:14 +0000
@@ -272,7 +272,7 @@
through the printed attribute accessors, uses the attributes of the
content class.
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
>>> print test_tales('context/badges:small', context=foo)
Foo.bugs
Foo.blueprints
=== modified file 'lib/canonical/launchpad/doc/displaying-numbers.txt'
--- lib/canonical/launchpad/doc/displaying-numbers.txt 2009-10-21 17:41:20 +0000
+++ lib/canonical/launchpad/doc/displaying-numbers.txt 2011-06-23 13:20:14 +0000
@@ -1,6 +1,6 @@
= Displaying Numbers with ZPT =
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
== bytes: Byte contractions ==
=== modified file 'lib/canonical/launchpad/doc/menus.txt'
--- lib/canonical/launchpad/doc/menus.txt 2011-03-23 16:28:51 +0000
+++ lib/canonical/launchpad/doc/menus.txt 2011-06-23 13:20:14 +0000
@@ -700,7 +700,7 @@
>>> from zope.publisher.interfaces.browser import IBrowserRequest
>>> from zope.publisher.interfaces.http import IHTTPApplicationRequest
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
>>> from canonical.launchpad.webapp import LaunchpadView
>>> from canonical.launchpad.webapp.vhosts import allvhosts
>>> class FakeRequest:
=== modified file 'lib/canonical/launchpad/doc/presenting-lengths-of-time.txt'
--- lib/canonical/launchpad/doc/presenting-lengths-of-time.txt 2010-10-18 22:24:59 +0000
+++ lib/canonical/launchpad/doc/presenting-lengths-of-time.txt 2011-06-23 13:20:14 +0000
@@ -3,7 +3,7 @@
First, let's bring in some dependencies:
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
>>> from datetime import timedelta
Exact Duration
=== modified file 'lib/canonical/launchpad/ftests/__init__.py'
--- lib/canonical/launchpad/ftests/__init__.py 2010-12-02 16:13:51 +0000
+++ lib/canonical/launchpad/ftests/__init__.py 2011-06-23 13:20:14 +0000
@@ -1,8 +1,25 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=W0401
+__all__ = [
+ 'ANONYMOUS',
+ 'decrypt_content',
+ 'import_public_key',
+ 'import_public_test_keys',
+ 'import_secret_test_key',
+ 'is_logged_in',
+ 'LaunchpadFormHarness',
+ 'login',
+ 'login_person',
+ 'logout',
+ 'print_date_attribute',
+ 'set_so_attr',
+ 'sync',
+ 'syncUpdate',
+ ]
+
from canonical.launchpad.ftests._launchpadformharness import (
LaunchpadFormHarness,
)
@@ -24,7 +41,4 @@
login,
login_person,
logout,
- test_tales,
)
-
-
=== modified file 'lib/canonical/lazr/doc/menus.txt'
--- lib/canonical/lazr/doc/menus.txt 2010-10-18 22:24:59 +0000
+++ lib/canonical/lazr/doc/menus.txt 2011-06-23 13:20:14 +0000
@@ -931,9 +931,9 @@
>>> from zope.interface import classImplements
>>> from zope.traversing.adapters import DefaultTraversable
>>> from zope.traversing.interfaces import IPathAdapter, ITraversable
- >>> from canonical.launchpad.ftests import test_tales
>>> from canonical.lazr.testing.menus import summarise_tal_links
>>> from lp.app.browser.tales import MenuAPI
+ >>> from lp.testing import test_tales
# MenuAPI is normally registered as an IPathAdapter in ZCML. This
# approximates what is done by the code:
=== modified file 'lib/devscripts/autoland.py'
--- lib/devscripts/autoland.py 2011-03-31 15:02:47 +0000
+++ lib/devscripts/autoland.py 2011-06-23 13:20:14 +0000
@@ -239,10 +239,6 @@
def get_email(person):
"""Get the preferred email address for 'person'."""
email_object = person.preferred_email_address
- # XXX: JonathanLange 2009-09-24 bug=319432: This raises a very obscure
- # error when the email address isn't set. e.g. with name12 in the sample
- # data. e.g. "httplib2.RelativeURIError: Only absolute URIs are allowed.
- # uri = tag:launchpad.net:2008:redacted".
return email_object.email
=== modified file 'lib/lp/app/browser/stringformatter.py'
--- lib/lp/app/browser/stringformatter.py 2011-05-29 23:34:26 +0000
+++ lib/lp/app/browser/stringformatter.py 2011-06-23 13:20:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""TALES formatter for strings."""
@@ -9,7 +9,9 @@
'add_word_breaks',
'break_long_words',
'escape',
+ 'extract_bug_numbers',
'FormattersAPI',
+ 'linkify_bug_numbers',
're_substitute',
'split_paragraphs',
]
@@ -170,6 +172,74 @@
return break_text_pat.sub(replace, text)
+def extract_bug_numbers(text):
+ '''Unique bug numbers matching the "LP: #n(, #n)*" pattern in the text.'''
+ # FormattersAPI._linkify_substitution requires a match object
+ # that has named groups "bug" and "bugnum". The matching text for
+ # the "bug" group is used as the link text and "bugnum" forms part
+ # of the URL for the link to the bug. Example:
+ # >>> bm.groupdict( )
+ # {'bugnum': '400686', 'bug': '#400686'}
+
+ # We need to match bug numbers of the form:
+ # LP: #1, #2, #3
+ # #4, #5
+ # over multiple lines.
+ #
+ # Writing a single catch-all regex for this has proved rather hard
+ # so I am taking the strategy of matching LP:(group) first, and
+ # feeding the result into another regex to pull out the bug and
+ # bugnum groups.
+ unique_bug_matches = dict()
+
+ line_matches = re.finditer(
+ 'LP:\s*(?P<buglist>(.+?[^,]))($|\n)', text,
+ re.DOTALL | re.IGNORECASE)
+
+ for line_match in line_matches:
+ bug_matches = re.finditer(
+ '\s*((?P<bug>#(?P<bugnum>\d+)),?\s*)',
+ line_match.group('buglist'))
+
+ for bug_match in bug_matches:
+ bugnum = bug_match.group('bugnum')
+ if bugnum in unique_bug_matches:
+ # We got this bug already, ignore it.
+ continue
+ unique_bug_matches[bugnum] = bug_match
+
+ return unique_bug_matches
+
+
+def linkify_bug_numbers(text):
+ """Linkify to a bug if LP: #number appears in the (changelog) text."""
+ unique_bug_matches = extract_bug_numbers(text)
+ for bug_match in unique_bug_matches.values():
+ replace_text = bug_match.group('bug')
+ if replace_text is not None:
+ # XXX julian 2008-01-10
+ # Note that re.sub would be far more efficient to use
+ # instead of string.replace() but this requires a regex
+ # that matches everything in one go. We're also at danger
+ # of replacing the wrong thing if string.replace() finds
+ # other matching substrings. So for example in the
+ # string:
+ # "LP: #9, #999"
+ # replacing #9 with some HTML would also interfere with
+ # #999. The liklihood of this happening is very, very
+ # small, however.
+ text = text.replace(
+ replace_text,
+ FormattersAPI._linkify_substitution(bug_match))
+ return text
+
+
+def extract_email_addresses(text):
+ '''Unique email addresses in the text.'''
+ matches = re.finditer(FormattersAPI._re_email, text)
+ return list(set([match.group() for match in matches]))
+
+
class FormattersAPI:
"""Adapter from strings to HTML formatted text."""
=== modified file 'lib/lp/app/browser/tests/test_stringformatter.py'
--- lib/lp/app/browser/tests/test_stringformatter.py 2011-05-29 23:34:26 +0000
+++ lib/lp/app/browser/tests/test_stringformatter.py 2011-06-23 13:20:14 +0000
@@ -15,7 +15,10 @@
from canonical.launchpad.testing.pages import find_tags_by_class
from canonical.launchpad.webapp.interfaces import ILaunchBag
from canonical.testing.layers import DatabaseFunctionalLayer
-from lp.app.browser.stringformatter import FormattersAPI
+from lp.app.browser.stringformatter import (
+ FormattersAPI,
+ linkify_bug_numbers,
+ )
from lp.testing import TestCase
@@ -141,6 +144,11 @@
expected_html,
[FormattersAPI(text).text_to_html() for text in test_strings])
+ def test_explicit_bug_linkification(self):
+ text = 'LP: #10'
+ self.assertEqual(
+ 'LP: <a href="/bugs/10">#10</a>', linkify_bug_numbers(text))
+
class TestLinkifyingProtocols(TestCase):
@@ -194,7 +202,7 @@
('<p><a rel="nofollow" '
'href="http://example.com/path_(with_parens">'
'http://<wbr></wbr>example.<wbr></wbr>com'
- '/path_<wbr></wbr>(with_parens</a></p>'),
+ '/path_<wbr></wbr>(with_parens</a></p>'),
]
self.assertEqual(
@@ -275,7 +283,6 @@
class TestDiffFormatter(TestCase):
"""Test the string formatter fmt:diff."""
- layer = DatabaseFunctionalLayer
def test_emptyString(self):
# An empty string gives an empty string.
=== modified file 'lib/lp/app/doc/displaying-dates.txt'
--- lib/lp/app/doc/displaying-dates.txt 2010-09-25 14:48:49 +0000
+++ lib/lp/app/doc/displaying-dates.txt 2011-06-23 13:20:14 +0000
@@ -27,7 +27,7 @@
First, let's bring in some dependencies:
>>> from datetime import datetime, timedelta
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
>>> import pytz
>>> UTC = pytz.timezone('UTC')
=== modified file 'lib/lp/app/doc/displaying-paragraphs-of-text.txt'
--- lib/lp/app/doc/displaying-paragraphs-of-text.txt 2010-12-21 16:13:15 +0000
+++ lib/lp/app/doc/displaying-paragraphs-of-text.txt 2011-06-23 13:20:14 +0000
@@ -6,7 +6,7 @@
== Basics ==
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
>>> text = ('This is a paragraph.\n'
... '\n'
=== modified file 'lib/lp/app/doc/tales-email-formatting.txt'
--- lib/lp/app/doc/tales-email-formatting.txt 2010-09-25 14:29:32 +0000
+++ lib/lp/app/doc/tales-email-formatting.txt 2011-06-23 13:20:14 +0000
@@ -10,7 +10,7 @@
First, let's bring in a small helper function:
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
== Quoting styles ==
=== modified file 'lib/lp/app/doc/tales-macro.txt'
--- lib/lp/app/doc/tales-macro.txt 2010-09-25 14:29:32 +0000
+++ lib/lp/app/doc/tales-macro.txt 2011-06-23 13:20:14 +0000
@@ -16,7 +16,7 @@
<html metal:use-macro="view/macro:page/main_side" />
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
>>> view = FakeView()
# Return value is the compiled macro expression.
=== modified file 'lib/lp/app/doc/tales.txt'
--- lib/lp/app/doc/tales.txt 2011-05-31 15:46:32 +0000
+++ lib/lp/app/doc/tales.txt 2011-06-23 13:20:14 +0000
@@ -7,7 +7,7 @@
First, let's bring in a small helper function:
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
The count: namespace to get numbers
=== modified file 'lib/lp/code/model/revision.py'
--- lib/lp/code/model/revision.py 2011-02-11 10:35:23 +0000
+++ lib/lp/code/model/revision.py 2011-06-23 13:20:14 +0000
@@ -345,7 +345,7 @@
# author per revision, so we use the first on the assumption that
# this is the primary author.
try:
- author = bzr_revision.get_apparent_authors()[0]
+ author = authors[0]
except IndexError:
author = None
return self.new(
=== modified file 'lib/lp/registry/browser/distributionsourcepackage.py'
--- lib/lp/registry/browser/distributionsourcepackage.py 2011-05-12 14:55:54 +0000
+++ lib/lp/registry/browser/distributionsourcepackage.py 2011-06-23 13:20:14 +0000
@@ -60,6 +60,10 @@
)
from lp.answers.enums import QuestionStatus
from lp.app.browser.tales import CustomizableFormatter
+from lp.app.browser.stringformatter import (
+ extract_bug_numbers,
+ extract_email_addresses,
+ )
from lp.app.enums import ServiceUsage
from lp.app.interfaces.launchpad import IServiceUsage
from lp.bugs.browser.bugtask import BugTargetTraversalMixin
@@ -78,8 +82,6 @@
from lp.registry.interfaces.series import SeriesStatus
from lp.services.propertycache import cachedproperty
from lp.soyuz.browser.sourcepackagerelease import (
- extract_bug_numbers,
- extract_email_addresses,
linkify_changelog,
)
from lp.soyuz.interfaces.archive import IArchiveSet
=== modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
--- lib/lp/registry/browser/tests/milestone-views.txt 2011-03-04 00:55:49 +0000
+++ lib/lp/registry/browser/tests/milestone-views.txt 2011-06-23 13:20:14 +0000
@@ -10,7 +10,7 @@
The default url for a milestone is to the main site.
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
>>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
>>> request = LaunchpadTestRequest(SERVER_URL='http://bugs.launchpad.net')
=== modified file 'lib/lp/registry/doc/sourcepackage.txt'
--- lib/lp/registry/doc/sourcepackage.txt 2011-04-11 16:00:53 +0000
+++ lib/lp/registry/doc/sourcepackage.txt 2011-06-23 13:20:14 +0000
@@ -383,19 +383,8 @@
<BLANKLINE>
The view will linkify bug numbers of the format "LP: #number" in the
-changelog if number is a valid bug ID (also see Soyuz page tests 24
--sourcepackage-changelog.txt). The changelog() method calls a method
-_linkify_bug_numbers() to do this.
-
-Zero or more spaces may appear between the ":" and the "#".
-
- >>> from lp.soyuz.browser.sourcepackagerelease import (
- ... linkify_bug_numbers)
- >>> linkify_bug_numbers("LP: #10")
- 'LP: <a href="/bugs/10">#10</a>'
-
-More complete examples of the bug linkification can be found in the doctest
-displaying-paragraphs-of-text.
+changelog if number is a valid bug ID (see Soyuz page tests 24
+-sourcepackage-changelog.txt).
Comparing Sourcepackages
=== modified file 'lib/lp/registry/doc/team-nav-menus.txt'
--- lib/lp/registry/doc/team-nav-menus.txt 2011-01-04 16:08:57 +0000
+++ lib/lp/registry/doc/team-nav-menus.txt 2011-06-23 13:20:14 +0000
@@ -3,7 +3,7 @@
The team pages have their own navigation menu.
>>> from zope.component import queryAdapter
- >>> from canonical.launchpad.ftests import test_tales
+ >>> from lp.testing import test_tales
>>> from canonical.launchpad.webapp.interfaces import INavigationMenu
>>> from canonical.lazr.testing.menus import summarise_tal_links
>>> from lp.registry.interfaces.person import IPersonSet
=== modified file 'lib/lp/scripts/utilities/warninghandler.py'
--- lib/lp/scripts/utilities/warninghandler.py 2010-08-20 20:31:18 +0000
+++ lib/lp/scripts/utilities/warninghandler.py 2011-06-23 13:20:14 +0000
@@ -158,9 +158,7 @@
if file is None:
file = sys.stderr
stream = StringIO.StringIO()
- # XXX: JonathanLange 2010-07-27: When Launchpad ceases supporting Python
- # 2.5, pass on the optional 'line' parameter.
- old_show_warning(message, category, filename, lineno, stream)
+ old_show_warning(message, category, filename, lineno, stream, line=line)
warning_message = stream.getvalue()
important_info = find_important_info()
=== modified file 'lib/lp/services/osutils.py'
--- lib/lp/services/osutils.py 2011-06-21 16:47:51 +0000
+++ lib/lp/services/osutils.py 2011-06-23 13:20:14 +0000
@@ -6,6 +6,7 @@
__metaclass__ = type
__all__ = [
'ensure_directory_exists',
+ 'get_pid_from_file',
'kill_by_pidfile',
'open_for_writing',
'override_environ',
=== modified file 'lib/lp/soyuz/browser/sourcepackagerelease.py'
--- lib/lp/soyuz/browser/sourcepackagerelease.py 2010-12-17 19:16:54 +0000
+++ lib/lp/soyuz/browser/sourcepackagerelease.py 2011-06-23 13:20:14 +0000
@@ -1,15 +1,11 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Browser view for a sourcepackagerelease"""
__metaclass__ = type
-# XXX: JonathanLange 2010-01-06: Many of these functions should be moved to a
-# generic lp.services.text module.
__all__ = [
- 'extract_bug_numbers',
- 'extract_email_addresses',
'linkify_changelog',
'SourcePackageReleaseView',
]
@@ -18,75 +14,10 @@
import re
from canonical.launchpad.webapp import LaunchpadView
-from lp.app.browser.stringformatter import FormattersAPI
-
-
-def extract_bug_numbers(text):
- '''Unique bug numbers matching the "LP: #n(, #n)*" pattern in the text.'''
- # FormattersAPI._linkify_substitution requires a match object
- # that has named groups "bug" and "bugnum". The matching text for
- # the "bug" group is used as the link text and "bugnum" forms part
- # of the URL for the link to the bug. Example:
- # >>> bm.groupdict( )
- # {'bugnum': '400686', 'bug': '#400686'}
-
- # We need to match bug numbers of the form:
- # LP: #1, #2, #3
- # #4, #5
- # over multiple lines.
- #
- # Writing a single catch-all regex for this has proved rather hard
- # so I am taking the strategy of matching LP:(group) first, and
- # feeding the result into another regex to pull out the bug and
- # bugnum groups.
- unique_bug_matches = dict()
-
- line_matches = re.finditer(
- 'LP:\s*(?P<buglist>(.+?[^,]))($|\n)', text,
- re.DOTALL | re.IGNORECASE)
-
- for line_match in line_matches:
- bug_matches = re.finditer(
- '\s*((?P<bug>#(?P<bugnum>\d+)),?\s*)',
- line_match.group('buglist'))
-
- for bug_match in bug_matches:
- bugnum = bug_match.group('bugnum')
- if bugnum in unique_bug_matches:
- # We got this bug already, ignore it.
- continue
- unique_bug_matches[bugnum] = bug_match
-
- return unique_bug_matches
-
-
-def linkify_bug_numbers(text):
- """Linkify to a bug if LP: #number appears in the (changelog) text."""
- unique_bug_matches = extract_bug_numbers(text)
- for bug_match in unique_bug_matches.values():
- replace_text = bug_match.group('bug')
- if replace_text is not None:
- # XXX julian 2008-01-10
- # Note that re.sub would be far more efficient to use
- # instead of string.replace() but this requires a regex
- # that matches everything in one go. We're also at danger
- # of replacing the wrong thing if string.replace() finds
- # other matching substrings. So for example in the
- # string:
- # "LP: #9, #999"
- # replacing #9 with some HTML would also interfere with
- # #999. The liklihood of this happening is very, very
- # small, however.
- text = text.replace(
- replace_text,
- FormattersAPI._linkify_substitution(bug_match))
- return text
-
-
-def extract_email_addresses(text):
- '''Unique email addresses in the text.'''
- matches = re.finditer(FormattersAPI._re_email, text)
- return list(set([match.group() for match in matches]))
+from lp.app.browser.stringformatter import (
+ FormattersAPI,
+ linkify_bug_numbers,
+ )
def obfuscate_email(user, text):
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2011-06-21 14:04:50 +0000
+++ lib/lp/testing/__init__.py 2011-06-23 13:20:14 +0000
@@ -154,8 +154,6 @@
with_celebrity_logged_in,
with_person_logged_in,
)
-# canonical.launchpad.ftests expects test_tales to be imported from here.
-# XXX: JonathanLange 2010-01-01: Why?!
from lp.testing._tales import test_tales
from lp.testing._webservice import (
api_url,
