[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/contributions-linkify-bugs into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/contributions-linkify-bugs into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cjwatson/launchpad/contributions-linkify-bugs/+merge/89403 I often look at https://dev.launchpad.net/Contributions when I'm trying to find a reference to some bit of work I did recently. It would be easier to navigate if bug numbers in commit messages were rendered as links. This branch makes it so. -- https://code.launchpad.net/~cjwatson/launchpad/contributions-linkify-bugs/+merge/89403 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/contributions-linkify-bugs into lp:launchpad. === modified file 'utilities/community-contributions.py' --- utilities/community-contributions.py 2012-01-01 03:10:25 + +++ utilities/community-contributions.py 2012-01-20 10:00:34 + @@ -305,6 +305,10 @@ class ContainerRevision(): A wrapper for a top-level LogRevision containing child LogRevisions. +# Regular expression based on that used by qa-tagger. +fixed_bug_re = re.compile( +r'(\[bugs?=)(\d+(?:,\s*\d+)*)(\])', re.IGNORECASE) + def __init__(self, top_lr, branch_info): Create a new ContainerRevision. @@ -320,10 +324,21 @@ Add a descendant child of this container revision. self.contained_revs.append(lr) +def format_fixed_bug_ids(self, text): +Format any bug numbers in the given string as links. +def format_bugs(match): +return ( +match.group(1) + +re.sub(r'(\d+)', r'[[Bug:\1]]', match.group(2)) + +match.group(3)) + +return self.fixed_bug_re.sub(format_bugs, text) + def __str__(self): timestamp = self.top_rev.rev.timestamp timezone = self.top_rev.rev.timezone message = self.top_rev.rev.message or (NO LOG MESSAGE) +message = self.format_fixed_bug_ids(message) rev_id = self.top_rev.rev.revision_id or (NO REVISION ID) if timestamp: date_str = format_date(timestamp, timezone or 0, 'original') ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/contributions-linkify-bugs into lp:launchpad
The proposal to merge lp:~cjwatson/launchpad/contributions-linkify-bugs into lp:launchpad has been updated. Status: Needs review = Merged For more details, see: https://code.launchpad.net/~cjwatson/launchpad/contributions-linkify-bugs/+merge/89403 -- https://code.launchpad.net/~cjwatson/launchpad/contributions-linkify-bugs/+merge/89403 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/contributions-linkify-bugs into lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/contributions-linkify-bugs into lp:launchpad
Review: Needs Fixing
... except {{{ }}} ignores markup, and all the workarounds wgrant and I have
tried are painful in some way. Let's just shelve this for now.
--
https://code.launchpad.net/~cjwatson/launchpad/contributions-linkify-bugs/+merge/89403
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~cjwatson/launchpad/contributions-linkify-bugs into lp:launchpad.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/kill-old-switchdbuser into lp:launchpad
The proposal to merge lp:~cjwatson/launchpad/kill-old-switchdbuser into lp:launchpad has been updated. Commit Message changed to: Migrate everything to the functions in lp.testing.dbuser, and kill off the old LaunchpadZopelessLayer.switchDbUser method. For more details, see: https://code.launchpad.net/~cjwatson/launchpad/kill-old-switchdbuser/+merge/89459 -- https://code.launchpad.net/~cjwatson/launchpad/kill-old-switchdbuser/+merge/89459 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/kill-old-switchdbuser into lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] lp:~abentley/launchpad/bugcomment-as-icomment into lp:launchpad
Aaron Bentley has proposed merging
lp:~abentley/launchpad/bugcomment-as-icomment into lp:launchpad with
lp:~abentley/launchpad/comment-lint as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~abentley/launchpad/bugcomment-as-icomment/+merge/89497
= Summary =
Truncate long code review comments and provide Read more... link.
== Proposed fix ==
Use common rendering for BugComment and CodeReviewComment
== Pre-implementation notes ==
Discussed with deryck
== Implementation details ==
Provide MessageComment to implement IComment in terms of IMessage, and use in
CodeReivewComment, BugComment, DistroSeriesDifferenceDisplayComment. Provide
adapters to convert CodeReviewComment and DistroSeriesDifferenceDisplayComment
into IMessage.
Move the code for truncation/Read More from bug comments to
lp.services.comments.
IComment gains too_long and text_for_display attributes. MessageComment
implements them.
Support truncation to 3200 chars in CodeReviewDisplayComment.
Adjust CodeReviewDisplayComment call sites to specify whether to truncate.
Use a single comment-text css class, instead of multiple ones.
Add browser_open to harness.py, to simplify interactive testing.
== Tests ==
bin/test -t test_bugcomment -t bugcomment.txt -t xx-bug-comments-truncated.txt
-t xx-bug-index-lots-of-comments.txt -t xx-numbered-comments.txt -t
test_branchmergeproposal -t test_codereviewcomment -t conversation.txt
== Demo and Q/A ==
Create a code review comment longer than 3200 chars. It should be truncated
and have a Read more link. That link should display the full code review
comment.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/services/comments/browser/configure.zcml
lib/lp/registry/browser/configure.zcml
lib/lp/bugs/browser/tests/test_bugcomment.py
lib/lp/code/browser/configure.zcml
lib/lp/bugs/stories/bugs/xx-bug-comments-truncated.txt
lib/lp/code/browser/branch.py
lib/lp/code/templates/codereviewcomment-body.pt
lib/canonical/launchpad/icing/css/modifiers.css
lib/lp/code/browser/tests/test_branchmergeproposal.py
lib/lp/code/browser/tests/test_codereviewcomment.py
lib/lp/bugs/browser/bugcomment.py
lib/lp/services/comments/browser/messagecomment.py
lib/lp/registry/browser/distroseriesdifference.py
lib/lp/testing/factory.py
lib/lp/services/comments/interfaces/conversation.py
lib/lp/code/browser/codereviewcomment.py
lib/lp/bugs/interfaces/bugmessage.py
lib/lp/scripts/harness.py
lib/lp/bugs/stories/bugs/xx-numbered-comments.txt
lib/lp/services/comments/templates/comment-body.pt
lib/lp/bugs/doc/bugcomment.txt
lib/lp/services/comments/doc/conversation.txt
lib/lp/code/browser/branchmergeproposal.py
lib/lp/bugs/templates/bugcomment-box.pt
lib/lp/bugs/stories/bugs/xx-bug-index-lots-of-comments.txt
./lib/canonical/launchpad/icing/css/modifiers.css
61: Unknown Property name.: filter
62: Unknown Property name.: -ms-filter
140: Unknown Property name.: filter
141: Unknown Property name.: -ms-filter
61: I009: Wrong separator on property: value pair.
140: I009: Wrong separator on property: value pair.
./lib/lp/scripts/harness.py
26: 'from storm.expr import *' used; unable to detect undefined names
28: 'from storm.locals import *' used; unable to detect undefined names
21: 'rlcompleter' imported but unused
--
https://code.launchpad.net/~abentley/launchpad/bugcomment-as-icomment/+merge/89497
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~abentley/launchpad/bugcomment-as-icomment into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/css/modifiers.css'
--- lib/canonical/launchpad/icing/css/modifiers.css 2012-01-20 21:22:32 +
+++ lib/canonical/launchpad/icing/css/modifiers.css 2012-01-20 21:22:32 +
@@ -155,9 +155,8 @@
pre.changelog,
table.diff,
-.bug-comment,
-.bug-activity,
-.codereviewcomment {
+.comment-text,
+.bug-activity {
font-family: 'Ubuntu Mono', monospace;
}
=== modified file 'lib/lp/bugs/browser/bugcomment.py'
--- lib/lp/bugs/browser/bugcomment.py 2012-01-06 19:45:52 +
+++ lib/lp/bugs/browser/bugcomment.py 2012-01-20 21:22:32 +
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2006-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
Bug comment browser view classes.
@@ -41,6 +41,7 @@
from lp.bugs.interfaces.bugattachment import BugAttachmentType
from lp.bugs.interfaces.bugmessage import IBugComment
+from lp.services.comments.browser.messagecomment import MessageComment
from lp.services.config import config
from lp.services.features import getFeatureFlag
from lp.services.librarian.browser import ProxiedLibraryFileAlias
@@ -176,7 +177,7 @@
yield
[Launchpad-reviewers] [Merge] lp:~sinzui/launchpad/dsp-vocab-fixes into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/dsp-vocab-fixes into
lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/dsp-vocab-fixes/+merge/89505
Allow users to find official unpublished packages in the target picker.
Launchpad bug: https://bugs.launchpad.net/bugs/919413
Pre-implementation: no one
On qastaging, where the DSP vocabulary is enabled, I expect to search
for charms/mysql and get an exact match when choosing a package affected
by a bug. The picker says there are no matches.
The problem is obvious looking at the implementation, the SQL joins to
DistributionSourcePackageCache to get rich package information, but
unpublished packages will never appear in the table. This is a
palm-in-face moment for me because 1. I insisted we extend DSP to do
searches because official packages may never be published, yet 2, I
wrote the SQL query that joins to a table predicated on publishing. The
query/scoring of results should use SourcePackageName.name instead of
dspc.name because the former is guaranteed to exist
RULES
* Join on spn, left join to dspc.
* Use the spn.name in scoring
QA
* Visit https://bugs.qastaging.launchpad.net/launchpad/+bug/741639
* Expand the affects row.
* Choose to pick a package.
* Search for 'charms/mysql' (this is an official unpublished package)
* Verify the results show a match, but there is no binary packages
listed in the description
LINT
lib/lp/registry/vocabularies.py
lib/lp/registry/tests/test_dsp_vocabularies.py
TEST
./bin/test -vv -t lp.registry.tests.test_dsp_vocabularies
IMPLEMENTATION
The addition of spn and changing dspc was near trivial. The test passed
after a fix of a few typos. The second test fix took much longer because
I was certain the test was correct and my SQL changes were bad. The test
was flawed. It created *two* packages, on official unpublished package
and a second unofficial package in a PPA. It was passing because of the
joins, but it should have failed because there was a matching official
package. The correction is to ensure that only one unofficial package is
created.
lib/lp/registry/vocabularies.py
lib/lp/registry/tests/test_dsp_vocabularies.py
--
https://code.launchpad.net/~sinzui/launchpad/dsp-vocab-fixes/+merge/89505
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~sinzui/launchpad/dsp-vocab-fixes into lp:launchpad.
=== modified file 'lib/lp/registry/tests/test_dsp_vocabularies.py'
--- lib/lp/registry/tests/test_dsp_vocabularies.py 2012-01-01 02:58:52 +
+++ lib/lp/registry/tests/test_dsp_vocabularies.py 2012-01-20 22:17:24 +
@@ -221,6 +221,18 @@
self.assertEqual(1, len(terms))
self.assertEqual('fnord/snarf', terms[0].token)
+def test_searchForTerms_exact_unpublished_offcial_source_name(self):
+# Exact source name matches of unpublished packages are found.
+distribution = self.factory.makeDistribution(name='fnord')
+self.factory.makeDistributionSourcePackage(
+distribution=distribution, sourcepackagename='snarf',
+with_db=True)
+vocabulary = DistributionSourcePackageVocabulary(None)
+results = vocabulary.searchForTerms(query='fnord/snarf')
+terms = list(results)
+self.assertEqual(1, len(terms))
+self.assertEqual('fnord/snarf', terms[0].token)
+
def test_searchForTerms_similar_offcial_source_name(self):
# Partial source name matches are found.
self.factory.makeDSPCache('fnord', 'pting-snarf-ack')
@@ -325,7 +337,8 @@
def test_searchForTerms_ppa_archive(self):
# Packages in PPAs are ignored.
-self.factory.makeDSPCache('fnord', 'snarf', archive='ppa')
+self.factory.makeDSPCache(
+'fnord', 'snarf', official=False, archive='ppa')
vocabulary = DistributionSourcePackageVocabulary(None)
results = vocabulary.searchForTerms(query='fnord/snarf')
terms = list(results)
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2012-01-06 11:08:30 +
+++ lib/lp/registry/vocabularies.py 2012-01-20 22:17:24 +
@@ -2147,25 +2147,27 @@
SELECT dsp.id, dsps.name, dsps.binpkgnames, rank
FROM DistributionSourcePackage dsp
JOIN (
-SELECT DISTINCT ON (dspc.sourcepackagename)
-dspc.sourcepackagename, dspc.name, dspc.binpkgnames,
-CASE WHEN dspc.name = ? THEN 100
+SELECT DISTINCT ON (spn.id)
+spn.id, spn.name, dspc.binpkgnames,
+CASE WHEN spn.name = ? THEN 100
WHEN dspc.binpkgnames SIMILAR TO
'(^| )' || ? || '( |$)' THEN 75
-
[Launchpad-reviewers] [Merge] lp:~rharding/launchpad/resizing_textarea_919299 into lp:launchpad
Richard Harding has proposed merging
lp:~rharding/launchpad/resizing_textarea_919299 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #919299 in Launchpad itself: resizing textarea in chrome in MP and
blueprints inlineedit flip flops height
https://bugs.launchpad.net/launchpad/+bug/919299
For more details, see:
https://code.launchpad.net/~rharding/launchpad/resizing_textarea_919299/+merge/89515
= Summary =
There was an issue with the last bug fix that caused the 3rd change to a
resizing textarea to resize to the min height for the widget. This then was
reverted on the next resize event, and repeated over and over which has a
lovely bouncing textarea effect.
== Proposed Fix ==
Correct the condition when we should make sure the textarea changes itself to
the min height configured.
== Implementation Details ==
Basically the check should have been and not !=
== Tests ==
Added a new test to verify the condition
google-chrome lib/lp/app/javascript/tests/test_beta_notification.html
== Demo and Q/A ==
Enter enough text into an inline edit widget that it hits the max size. Then
make sure that it sticks there vs reverting between the max and min heights
configured.
== Lint ==
Linting changed files:
lib/lp/app/javascript/formwidgets/resizing_textarea.js
lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html
lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js
--
https://code.launchpad.net/~rharding/launchpad/resizing_textarea_919299/+merge/89515
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~rharding/launchpad/resizing_textarea_919299 into lp:launchpad.
=== modified file 'lib/lp/app/javascript/formwidgets/resizing_textarea.js'
--- lib/lp/app/javascript/formwidgets/resizing_textarea.js 2012-01-18 13:54:33 +
+++ lib/lp/app/javascript/formwidgets/resizing_textarea.js 2012-01-21 00:41:24 +
@@ -280,7 +280,7 @@
this.set_overflow();
this._prev_scroll_height = scroll_height;
-} else if (this._prev_scroll_height !== this.get('min_height')) {
+} else if (this._prev_scroll_height this.get('min_height')) {
// This can occur when we go from hidden to not hidden via a
// parent display setting. The initial height needs to get hit
// again manually on display.
=== modified file 'lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html'
--- lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html 2012-01-18 13:46:13 +
+++ lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html 2012-01-21 00:41:24 +
@@ -32,6 +32,7 @@
style=max-width: 60em; width: 90%; display: block; cols=44
/textarea
/div
+textarea id=no_change style=width: auto;/textarea
textarea id=one_line_sample style=height: 1em; overflow: hidden; resize: none; width: auto;/textarea
textarea id=one_line style=height: 1em; width: auto;/textarea
/body
=== modified file 'lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js'
--- lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js 2012-01-18 13:54:33 +
+++ lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js 2012-01-21 00:41:24 +
@@ -168,6 +168,32 @@
The height should start out at 300px per the min_height config);
},
+test_height_stays_consistant: function () {
+// once we adjust the height, another keystroke shouldn't move the
+// height on us again, see bug #919299
+var target = Y.one('#no_change');
+target.plug(Y.lp.app.formwidgets.ResizingTextarea, {
+skip_animations: true,
+max_height: 200,
+min_height: 100
+});
+
+update_content(target, test_text);
+var new_height = get_height(target);
+Y.Assert.areSame(200, new_height,
+The height should hit max at 200px);
+
+update_content(target, test_text + 3);
+var adjusted_height = get_height(target);
+Y.Assert.areSame(200, adjusted_height,
+The height should still be at 200px);
+
+update_content(target, test_text + 34);
+var adjusted_height2 = get_height(target);
+Y.Assert.areSame(200, adjusted_height2,
+The height should still be at 200px);
+},
+
test_oneline_should_size_to_single_em: function() {
// Passing a one line in the cfg should limit the height to 1em even
// though a normal textarea would be two lines tall.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
