Merge lp:~jamalta/launchpad/destination-ppa-498643 into lp:launchpad

Proposed by Jamal Fanaian
Status: Merged
Approved by: Brad Crittenden
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~jamalta/launchpad/destination-ppa-498643
Merge into: lp:launchpad
Diff against target: 58 lines (+6/-6)
3 files modified
lib/lp/soyuz/browser/archive.py (+1/-1)
lib/lp/soyuz/browser/tests/archive-views.txt (+3/-3)
lib/lp/soyuz/stories/ppa/xx-copy-packages.txt (+2/-2)
To merge this branch: bzr merge lp:~jamalta/launchpad/destination-ppa-498643
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+17628@code.launchpad.net

Commit message

Changed the Destination PPA options to read 'PPA Name (username/ppa)' in copy packages page to help distinguish between similarly named PPAs.

To post a comment you must log in.
Revision history for this message
Jamal Fanaian (jamalta) wrote :

= Summary =

Add PPA url to the Destination PPA widget when copying contents from a PPA, to make each option unique (bug 498643).

== Proposed fix ==

Appended the PPA url to each widget option:
  Name of PPA (user/ppaname)

== Pre-implementation notes ==

Following style suggestion by Jullian Edwards in the bug comments.

== Tests ==

% bin/test -vvct xx-copy-packages

== Demo and Q/A ==

Must login as user with an existing PPA (u: <email address hidden> p: test).

* https://launchpad.dev/~cprov/+archive/ppa/+copy-packages

== Launchpad lint ==

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/browser/archive.py
  lib/lp/soyuz/stories/ppa/xx-copy-packages.txt

Revision history for this message
Brad Crittenden (bac) wrote :

Hi Jamal,

Your fix looks good. In your diff at line 9, please change the line back to have a 4 space indentation. You can then bring the following line up as it will just be 77 characters.

Also you agreed to fix the import violation exposed by running your test. Thanks.

review: Approve (code)
Revision history for this message
Jamal Fanaian (jamalta) wrote :

Brad,

Thanks for the review. The suggested changes have been made, and the import violation was resolved.

Revision history for this message
Brad Crittenden (bac) wrote :

Running the full test suite on ec2 shows that lib/lp/soyuz/browser/tests/archive-views.txt must be updated too. Once that is done I will land the branch for you.

review: Needs Fixing (code)
Revision history for this message
Brad Crittenden (bac) wrote :

The tests now pass.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/soyuz/browser/archive.py'
2--- lib/lp/soyuz/browser/archive.py 2010-01-20 15:41:25 +0000
3+++ lib/lp/soyuz/browser/archive.py 2010-01-21 16:04:15 +0000
4@@ -1171,7 +1171,7 @@
5 continue
6 token = '%s/%s' % (ppa.owner.name, ppa.name)
7 terms.append(
8- SimpleTerm(ppa, token, ppa.displayname))
9+ SimpleTerm(ppa, token, '%s (%s)' % (ppa.displayname, token)))
10
11 return form.Fields(
12 Choice(__name__='destination_archive',
13
14=== modified file 'lib/lp/soyuz/browser/tests/archive-views.txt'
15--- lib/lp/soyuz/browser/tests/archive-views.txt 2010-01-12 08:03:29 +0000
16+++ lib/lp/soyuz/browser/tests/archive-views.txt 2010-01-21 16:04:15 +0000
17@@ -1218,7 +1218,7 @@
18
19 >>> for item in archive_widget.vocabulary:
20 ... print item.title
21- PPA for Ubuntu Team
22+ PPA for Ubuntu Team (ubuntu-team/ppa)
23
24 >>> print archive_widget.getInputValue() == cprov.archive
25 True
26@@ -1268,8 +1268,8 @@
27
28 >>> for item in archive_widget.vocabulary:
29 ... print item.title
30- PPA for Celso Providelo
31- PPA for No Privileges Person
32+ PPA for Celso Providelo (cprov/ppa)
33+ PPA for No Privileges Person (no-priv/ppa)
34
35 >>> print archive_widget.getInputValue()
36 Traceback (most recent call last):
37
38=== modified file 'lib/lp/soyuz/stories/ppa/xx-copy-packages.txt'
39--- lib/lp/soyuz/stories/ppa/xx-copy-packages.txt 2010-01-15 23:28:53 +0000
40+++ lib/lp/soyuz/stories/ppa/xx-copy-packages.txt 2010-01-21 16:04:15 +0000
41@@ -202,7 +202,7 @@
42
43 >>> print jblack_browser.getControl(
44 ... 'Destination PPA').displayOptions
45- ['PPA for James Blackwell']
46+ ['PPA for James Blackwell (jblack/ppa)']
47
48 >>> print jblack_browser.getControl(
49 ... 'Destination PPA').value
50@@ -698,7 +698,7 @@
51
52 >>> print jblack_browser.getControl(
53 ... 'Destination PPA').displayOptions
54- ['PPA for James Blackwell', 'PPA for James Blackwell Friends']
55+ ['PPA for James Blackwell (jblack/ppa)', 'PPA for James Blackwell Friends (jblack-friends/ppa)']
56
57 James wants to populate the PPA for James Blackwell Friends, he
58 selects that.