Ubuntu
checkbox package

Merge lp://qastaging/~roadmr/ubuntu/oneiric/checkbox/0.12.9-respin into lp://qastaging/ubuntu/oneiric/checkbox

  1. Oneiric (11.10)
  2. 0.12.9-respin
  3. Merge into oneiric
Proposed by Daniel Manrique
Status: Merged
Merge reported by: Mathieu Trudel-Lapierre
Merged at revision: not available
Proposed branch: lp://qastaging/~roadmr/ubuntu/oneiric/checkbox/0.12.9-respin
Merge into: lp://qastaging/ubuntu/oneiric/checkbox
Diff against target: 294 lines (+91/-55)
10 files modified
checkbox/contrib/persist.py (+5/-4)
checkbox/lib/safe.py (+5/-0)
checkbox/message.py (+4/-3)
checkbox_gtk/gtk_interface.py (+2/-4)
debian/changelog (+24/-0)
gtk/checkbox-gtk.ui (+37/-37)
jobs/bluetooth.txt.in (+6/-2)
jobs/input.txt.in (+1/-1)
plugins/persist_info.py (+2/-1)
scripts/usb_test (+5/-3)
To merge this branch: bzr merge lp://qastaging/~roadmr/ubuntu/oneiric/checkbox/0.12.9-respin
Reviewer Review Type Date Requested Status
Daniel Manrique (community) Needs Resubmitting
Mathieu Trudel-Lapierre Approve
Review via email: mp+87768@code.qastaging.launchpad.net

Description of the change

We submitted a Checkbox SRU (see https://code.launchpad.net/~roadmr/ubuntu/oneiric/checkbox/0.12.8.1/+merge/84660) containing 8 fixes backported from the development branch. Unfortunately two of those bugs failed verification: bug 862322 due to a dumb mistake on my part where I updated the changelog but neglected to add the code fix; and bug 877752, whose fix contains *another* bug which still exists in development.

Thus I'd like to withdraw the previous Checkbox SRU and submit this one instead. This adds the fix for bug 862322 and removes altogether the fix for bug 877752 (pending further investigation and publishing a fix in development). Those are the only changes with respect to the prior merge request.

I used the 0.12.9 version number as that's what the previous SRU was versioned to by Luke Yelavich, and targeted to oneiric-proposed.

Let me know if I need to do anything with the previous merge proposal.

Thanks and apologies for the churn on this SRU request!

To post a comment you must log in.
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Why are you dropping the require: alsa-utils for the bluetooth-audio test? It seems to me like alsa-utils is the only package providing aplay and arecord, so if it´s not available, this test would fail to run. Regardless, this is an extra change that does not appear in the changelog.

The rest looks fine to me :)

review: Needs Fixing
Revision history for this message
Daniel Manrique (roadmr) wrote :

Hi Mathieu,

I see that change was part of the bluetooth fixes from checkbox rev 1114, which I just merged "as it was" from development. I think it may have been an incomplete attempt to add requires: device.category == 'BLUETOOTH' like the rest of the changes in that commit. Impact should be minimal as alsa-utils is in the CD by default so the requires: as removed was just a safety net.

Any ideas on how I should handle this? I can either add requires: to match the other tests (though that is *not* in either trunk or development, so it'd be "original" code introduced in an SRU) or remove that commit entirely (which I'd rather not do, as then the other tests will run even for people without bluetooth - at least the bluetooth_audio test can be skipped).

Thanks so much for reviewing this code!

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

My personal feeling would be to re-add just that require, as-is. You're already depending on the other bluetooth tests for bluez and device.category checks; and the bluetooth tests require bluez which is also a package on the CD. Removing the requirement for alsa-utils, despite it being a corner-case, introduces the possible regression of failing tests if alsa-utils was removed (rather than keeping the current behavior of skpping the test). That and it could be a little hard to track down in a bug report :)

lp://qastaging/~roadmr/ubuntu/oneiric/checkbox/0.12.9-respin updated
39. By Daniel Manrique

re-add alsa-utils requirement for bluetooth/audio job as recommended by reviewer

Revision history for this message
Daniel Manrique (roadmr) wrote :

Mathieu,

I've added the requires: alsa-utils line to the bluetooth/audio job.

Thanks for your advice, much appreciated!

review: Needs Resubmitting
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Looks good, thanks for looking into this. Approved.

review: Approve
lp://qastaging/~roadmr/ubuntu/oneiric/checkbox/0.12.9-respin updated
40. By Daniel Manrique

rearrange changelog and request new version 0.12.10

Revision history for this message
Daniel Manrique (roadmr) wrote :

I added a new version to the changelog (0.12.10) and moved the change I'd forgotten from the previous version there.

The bug whose code I removed (bug 877752) was also removed from the changelog altogether, let me know if this needs to be indicated somehow on the changelog (i.e. an entry indicating code was removed).

Revision history for this message
Daniel Manrique (roadmr) :
review: Needs Resubmitting
Revision history for this message
Daniel Manrique (roadmr) wrote :

Bump... sorry to nudge on this. Does this need something else to be re-merged?

Thanks!

review: Needs Resubmitting

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches