[00:05] <wgrant> StevenK: You broke the build, btw
[00:05] <StevenK> Blah
[00:07] <StevenK> wgrant: I think I'll set summary_text to 'Rejected by archive administrator.' rather than None, which will solve one of the failures.
[00:09] <wgrant> StevenK: Or find any tests that don't pass a user and fix them
[00:09] <wgrant> There's probably not many
[00:11] <StevenK> You'd prefer summary_text to be None rather than a generic message in the case user and comment are None?
[00:13] <wgrant> StevenK: I'd prefer the user argument to be mandatory, since there's no reason for it to not be
[00:13] <wgrant> And given the few failures it seems like there are probably few tests
[00:13] <StevenK> wgrant: Well, that can be fixed pretty easily
[01:13] <StevenK> wgrant: http://pastebin.ubuntu.com/5708682/
[01:15] <wgrant> -            Rejected by archive administrator.
[01:15] <wgrant> +            None
[01:15] <wgrant> Otherwise that looks reasonable
[01:15] <StevenK> wgrant: That test is calling notify() directly
[01:16] <wgrant> StevenK: Ah, I'd consider passing in a summary_text, then
[01:16] <wgrant> Because no rejection email will normally have None for summary_text
[01:23] <StevenK> wgrant: The diff for that test is now:
[01:23] <StevenK>              person, None, [bpr], [], archive, distroseries, pocket,
[01:23] <StevenK> +            summary_text="Rejected by archive administrator.",
[01:23] <StevenK>              action='rejected')
[01:23] <wgrant> Great
[01:25]  * StevenK tosses it at buildbot
[03:50] <StevenK> iharness seems to want to grab __dict__ for any objects you ask it to print, which is very annoying
[04:14] <StevenK> Blah
[04:14] <StevenK> DistroSeries:+queue, stop lying!
[04:18] <StevenK> Ah ha, the archive is wrong
[04:18] <wgrant> What are you breaking?
[04:19] <StevenK> wgrant: Adding the textbox to DistroSeries:+queue
[04:21] <StevenK> wgrant: The template gets compiled by Zope, or I can change it without restarting the appserver?
[04:24] <wgrant> StevenK: Template changes don't require an appserver restart today
[04:25] <wgrant> (which is part of the reason that our current TAL renderer is so ridiculously slow)
[04:25] <StevenK> Hah
[04:25] <StevenK> Oh, damn it
[04:25] <StevenK> It's all one row
[04:25] <wgrant> Hm?
[04:25] <StevenK> With <br/>'s and vertical-align: bottom
[04:26] <wgrant> Oh lovely.
[04:37] <StevenK> Hmmm, this might work.
[04:37] <StevenK> wgrant: Do you want a diff of the template or a screenshot?
[04:38] <wgrant> StevenK: Why not both?
[04:40] <StevenK> wgrant: http://people.canonical.com/~stevenk/reject-reason-queue.jpg and  http://pastebin.ubuntu.com/5708984/
[04:42] <wgrant> StevenK: I'td say "Rejection comment" (widget captions aren't title case), and have Reject below Accept so the columns look less bad
[04:42] <wgrant> And maybe make the textbox wider, given there's so much space?
[04:43] <wgrant> Like I might put "Rejection comment" in the first two columns like "Override for selected uploads", and then have the textbox span the three override cols.
[04:46] <StevenK> wgrant: http://people.canonical.com/~stevenk/reject-reason-queue-1.jpg and http://pastebin.ubuntu.com/5708991/
[04:47] <wgrant> StevenK: The CSS on the label is different
[04:47] <wgrant> Also your GTK theme is fugly
[04:47] <StevenK> Meh
[04:49] <StevenK> wgrant: Refresh -1, that has the updated CSS. http://pastebin.ubuntu.com/5708997/
[04:50] <wgrant> Your GTK theme still sucks, but otherwise that looks fine :)
[04:50] <StevenK> Blah
[04:50] <StevenK> Now to wire it up
[04:51] <wgrant> Should be nice and easy.
[06:08] <StevenK> wgrant: https://code.launchpad.net/~stevenk/launchpad/reject-reason-plus-queue/+merge/165960
[06:09] <wgrant> StevenK: That's not an error message
[06:10] <wgrant> "Rejection comment required." maybe
[06:11] <wgrant> Also, that queue_action_* stuff seems entirely pointless. I'd inline it to be clearer.
[06:11] <wgrant> it's even shorter...
[06:11] <StevenK> wgrant: You'd rather an if action ?
[06:12] <wgrant> Yes
[06:12] <wgrant> there's only two cases, and I can't see us adding more
[06:12] <wgrant> The dynamic lookup almost makes sense for, say, build results
[06:12] <wgrant> But for accept/reject? Unnecessary complication.
[06:13] <wgrant> Also, I'd think assertStatus would take a PackagePublishingStatus, not a string, but either works.
[06:14] <StevenK> wgrant: I'm happy to switch to PackagePublishingStatus
[06:14] <wgrant> That would put it more in line with every other test in the codebase
[06:14] <wgrant> So I think it's a good idea
[06:16] <StevenK> ITYM PackageUploadStatus, but yeah
[06:16] <wgrant> Er, that, yeah
[06:16] <wgrant> I'm working with publication methods at the moment, can you tell? :)
[06:16] <StevenK> :-)
[06:23] <StevenK> wgrant: Will my new test class cause all tests in the super class to be run twice?
[06:24] <wgrant> StevenK: Yes
[06:24] <wgrant> I'd merge the two classes, probably
[06:24] <wgrant> Rather than extracting a base
[06:29] <StevenK> wgrant: http://pastebin.ubuntu.com/5709168/
[06:32] <wgrant> Looks good
[06:33] <StevenK> wgrant: Any other objections, or shall I push?
[06:33] <wgrant> Push!
[06:42] <StevenK> wgrant: The MP is updated
[06:46] <wgrant> StevenK: r=me
[09:47] <czajkowski> jtv:  Is there any way to set up rosetta on a branch owned by a public team?
[09:49] <jtv> Hi czajkowski — you mean to export translations to?  I think there have been changes in this area, so for the current state of affairs you'd best talk to the Australians.
[09:49] <jtv> The situation when I worked with it was that you had to own the branch in order to select it, but then of course you could change its ownership.
[09:50] <czajkowski> jtv: cool thanks wgrant has asked for clarification from the user in lp land
[09:50] <czajkowski> thanks