launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-800573 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #800573 in Launchpad itself: "Need icon for sync package uploads"
  https://bugs.launchpad.net/launchpad/+bug/800573

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-800573/+merge/69240

= Summary =

"Sync" uploads on the DistroSeries:+queue page need an identifying icon, just like source uploads and uploads with binaries attached etc. have them.  (These aren't all mutually exclusive, so it's really more of a classifying icon list than an identifying icon.)

Huw "Emo" Wilkins has provided us with a nice sync icon, included here.


== Proposed fix ==

The code for this is already completely generic, so it's just a matter of adding the icon's image file to the source tree, and adding a tuple to the list of possible icons for an upload.


== Pre-implementation notes ==

We discussed the possibility of making this a sprite.  But Huw tells us the sprites image is already getting a bit bigger than we'd like.  Anyway it would complicate the code, unless we could make all of these icons sprites.  I suggested the idea of a separate sprites sheet for future consideration, but stuck with a loose image file for now.


== Implementation details ==

I verified manually that the icon shows up for the right type of upload (and that it doesn't for the wrong type, and that the other types still work as expected), but I haven't written an end-to-end test to prove it.  Guilty though I feel about it, I don't think there's much point.  We don't verify this for our other images, even in browser tests.

However there are tests that exercise all the code in this path, so we know it's not going to break anything.


== Tests ==

{{{
./bin/test -vvc lp.soyuz.browser.tests.test_queue -t composeIconList
}}}


== Demo and Q/A ==

Just what I did on my dev system, but on dogfood: see that the icon shows up on sync uploads, see that it doesn't on other uploads, and see that the other uploads still have their own icons.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/browser/queue.py
-- 
https://code.launchpad.net/~jtv/launchpad/bug-800573/+merge/69240
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-800573 into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/queue.py'
--- lib/lp/soyuz/browser/queue.py	2011-06-29 07:25:18 +0000
+++ lib/lp/soyuz/browser/queue.py	2011-07-26 10:21:33 +0000
@@ -557,6 +557,7 @@
         potential_icons = [
             (self.contains_source, ("Source", 'package-source')),
             (self.contains_build, ("Build", 'package-binary', "Binary")),
+            (self.package_copy_job, ("Sync", 'package-sync')),
             (self.contains_translation, ("Translation", 'translation-file')),
             (self.contains_installer, ("Installer", 'ubuntu-icon')),
             (self.contains_upgrader, ("Upgrader", 'ubuntu-icon')),