[06:17] <al-maisan> Good morning!
[09:56] <jtv> henninge: care to review?  
[09:56] <jtv> https://code.edge.launchpad.net/~jtv/launchpad/bug-440445/+merge/12777
[10:03] <henninge> jtv: so you are assuming that it will also need access to language and translator?
[10:04] <jtv> henninge: yes; it's looking at TranslationGroup to check for membership, and that definitely involves Translator.
[10:04] <henninge> jtv: valid assumption, I guess.
[10:05] <henninge> jtatum: r=me
[10:05] <jtv> Not sure about Language, but it's one of those things that are likely to be fetched by the ORM as a side product
[10:05] <jtv> henninge: no, the other person.  :)  Thanks.
[10:05] <henninge> jtv: r=me
[10:05] <jtv> :)
[10:05] <henninge> jtatum: whatever branch you want reviewed, I have *not* approved it! ;-)
[10:05] <henninge> jtatum: sorry to bother you ...
[10:07] <henninge> jtv: please remember to set the mp to "approved". For some reason I cannot do it.
[10:07] <jtv> henninge: sure, thanks
[10:19] <al-maisan> hello adeuring -- could you please have a look at a small/simple branch?
[10:19] <al-maisan> it is here: https://code.launchpad.net/~al-maisan/launchpad/cmsg-425800/+merge/12774
[10:19] <adeuring> al-maisan: I'm already doing this :)
[10:19] <al-maisan> adeuring: ah, wow!
[10:20] <al-maisan> adeuring: I am duly impressed :)
[10:29] <adeuring> al-maisan: there are two new tests: test_requestMirror_doesnt_demote_branch() and test_requestMirror_can_promote_branch(). Perhaps I'm missing something obvoius there -- but isn't future_time in the latter test a timestanp in the past: future_time = datetime.now(pytz.UTC) - timedelta(days=1) ?
[10:29]  * al-maisan looks
[10:30] <al-maisan> adeuring: argghh
[10:30] <al-maisan> the diff generated for the MP is wrong
[10:30] <al-maisan> just a minute
[10:31] <al-maisan> adeuring: http://pastebin.com/m4c94727b
[10:32] <al-maisan> sorry about that
[10:32] <al-maisan> this is a *really* simple branch
[10:32] <al-maisan> bbiab
[10:32] <adeuring> al-maisan: Ah, indeed quite much shorter ;)
[10:32] <adeuring> al-maisan: but the other test is there nevertheless ;)
[10:33] <al-maisan> adeuring: aha
[10:34] <al-maisan> adeuring: I don't see any of that in the diff I provided you with
[10:34]  * al-maisan rubs his eyes
[10:34] <adeuring> al-maisan: what I menat: I am simply curious about this test, even it is is not part of the patch.
[10:35] <al-maisan> adeuring: yep, 'future_time' would have to lie in the past .. provided the time on the test machine is set correctly :)
[10:37]  * adeuring is quite confused... future time in the past?
[10:37] <adeuring> al-maisan: but let's put that aside.
[10:38] <adeuring> al-maisan: al-maisan: just one question about the short diff: You removed the text "Did you mean to upload to a PPA?". Isn't this sometimes a useful hint for users?
[10:38] <al-maisan> adeuring: not really .. it would rather confuse people
[10:38] <adeuring> al-maisan: OK, so r=me
[10:39] <al-maisan> adeuring: thanks
[10:41] <allenap> adeuring: Fancy a devscripts.ec2test branch to review?
[10:41] <allenap> https://code.edge.launchpad.net/~allenap/launchpad/ec2-test-race-bug-422433/+merge/12778
[10:41] <adeuring> allenap: sure
[10:41] <allenap> adeuring: Thanks :)
[10:51] <bigjools> al-maisan: don't remove that error text
[11:03] <al-maisan> bigjools: alright; what do you suggest instead?
[11:03] <bigjools> al-maisan: I don't have any context - what are you doing?
[11:04] <al-maisan> bigjools: https://bugs.edge.launchpad.net/soyuz/+bug/425800
[11:04] <mup> Bug #425800: confusing message if signer can upload packages from some package sets but not others <packagesets> <soyuz-upload> <Soyuz:In Progress by al-maisan> <https://launchpad.net/bugs/425800>
[11:05] <bigjools> al-maisan: that error text is still correct, but only if the uploader has no packagesets
[11:05] <bigjools> he can upload to
[11:06] <al-maisan> bigjools: that's not quite right.
[11:06] <al-maisan> this error message indicates that the uploader has no upload permissions whatsoever
[11:06] <bigjools> al-maisan: yes, that's exactly what I mean
[11:07] <bigjools> so the existing check for that error text needs to also check packagesets
[11:07] <al-maisan> however, he may end up in this situation if he has *some* package set based permissions but is lacking one
[11:07] <bigjools> no, that is wrong
[11:08] <al-maisan> hmm .. I looked at the code .. the source package based permissions (including package sets) are checked before this error is raised
[11:09] <bigjools> the error should be raised if the upload has no component-based *and* no packageset-based permissions
[11:09] <al-maisan> is that error (formulated that way) useful in first place?
[11:10] <bigjools> yes - it distinguishes between NO permissions and SOME permissions but the wrong ones
[11:10] <al-maisan> aha
[11:10] <bigjools> PPA packages fall foul of it a lot, hence the extra text
[11:11]  * al-maisan takes another look at verify_upload()
[11:12] <bigjools> al-maisan: basically if the uploader is trying to upload to say, main, but only has universe permissions, this error is not used
[11:12] <bigjools> the same needs to apply with packagesets
[11:12] <al-maisan> bigjools: I see what you mean.
[11:12] <bigjools> great
[11:50] <adeuring> allenap: r=me
[11:50] <allenap> adeuring: Thanks :)
[11:51] <adeuring> allenap: what I forgot to say iin the review: thanks for splitting the diff into two parts!
[11:52] <allenap> adeuring: I'm glad it helped :)
[13:55] <allenap> adeuring: I replied to your review of ec2-test-race-bug-422433 with an incremental diff. Could you sanity check it, then I'll send it all off to ec2? Thanks.
[14:02] <adeuring> allenap: looks good. Thanks for the change!
[14:02] <allenap> adeuring: Thanks.
[14:19] <noodles775> barry, have you checked some bug pages also? The "Bug #440220 reported by Barry Warsaw 16 hours ago" is also using the registration slot iirc...
[14:19] <mup> Bug #440220: registering slot interferes with watermark <post-ui-3-cleanup> <Launchpad Registry:In Progress by barry> <https://launchpad.net/bugs/440220>
[14:19] <noodles775> which means that it will *always* wrap (unless you've got a 2m screen ;) ).
[14:20] <barry> noodles775: right.  let me paste a bug page s/s
[14:23] <barry> noodles775: attached two more s/s.  they do always wrap but it doesn't look too bad to me
[14:26] <noodles775> barry, I think the thing I don't like is that it's wrapping when it doesn't need to.
[14:27] <noodles775> Have you tried leaving it where it was after adding the no_wrap on the app-tabs?
[14:28] <barry> noodles775: it will run into the tabs if the registering contents are long (long person name).  maybe that's rare enough to not worry about it though?
[14:30] <noodles775> barry, if you give the registration slot a large left-margin (maybe 2em) it shouldn't get too close? (and will wrap since the app tabs can't, iirc?)
[14:30] <barry> noodles775: e.g. this does not look as good to me: http://launchpadlibrarian.net/32862698/ff-nowrap.png
[14:31] <barry> noodles775: that's a possibility too.  i kind of like that the registering information sits neatly above the portlet
[14:33] <noodles775> barry, hmm.... to me it often (ie. with longer slots like on bugs pages) looks like it's been squashed into a space that's too small.
[14:34] <noodles775> barry, I personally agree with rockstar's comment - if it was just below the app tabs, it wouldn't need to wrap at all, and would not interfere... but maybe there's a reason for not doing that?
[14:34] <barry> noodles775: it is a pretty small space (21%).  
[14:34] <barry> noodles775: i definitely want to explore that, but iirc beuno really wants that registering slot on the right side
[14:35] <barry> noodles775: so i kind of wanted to run that to ground to see if we could make it work
[14:35] <noodles775> barry, it would be on the right side wouldn't it? I just mean a little bit down from where it is in the last link above.
[14:36] <barry> noodles775: hmm... possibly so.  give me a few minutes and i'll try that
[14:36] <noodles775> k
[14:36] <barry> btw, thanks!
[14:37] <noodles775> np!
[14:48] <barry> noodles775: take a look at the s/s in comments 13-16
[14:51] <noodles775> barry, what do you think? My first thought is that the only thing that looks ugly about it is the super long username?
[14:53] <barry> noodles775: agreed.  i just did that to see worst case.  this works for me
[14:55] <noodles775> barry, great. Thanks for fixing it!
[14:55] <barry> noodles775: thanks!  can i request you as one of the two ui reviewers for the branch?
[14:55] <noodles775> barry, sure!
[14:55] <barry> awesome
[15:07] <barry> adeuring: ping
[15:07] <adeuring> barry: yes?
[15:07] <barry> adeuring: have time for a 39 line CSS review?
[15:07] <adeuring> barry: sure
[15:07] <barry> cool.  sending mp now
[15:20] <adeuring> barry: I have also an MP (a bit larger than yours, but just trivial refactoring): https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-refactor-haldevice-3/+merge/12733 could you review it?
[15:26] <barry> adeuring: sure!  this feels so incestuous though :)
[15:26] <adeuring> barry: ;)
[15:33] <adeuring> barry: r=me
[16:06] <barry> adeuring: thanks!
[16:11] <barry> adeuring: did you request me as a reviewer on your branch?
[16:11] <adeuring> barry: no
[16:11] <barry> adeuring: url?
[16:11] <adeuring> barry: https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-refactor-haldevice-3/+merge/12733
[16:22] <al-maisan> hello adeuring, can you please have a look at the fix for bug #425800 (2nd try) ?
[16:22] <mup> Bug #425800: confusing message if signer can upload packages from some package sets but not others <packagesets> <soyuz-upload> <Soyuz:In Progress by al-maisan> <https://launchpad.net/bugs/425800>
[16:22] <al-maisan> https://code.launchpad.net/~al-maisan/launchpad/cmsg-2-425800/+merge/12791
[16:22] <adeuring> al-maisan: sure
[16:22] <al-maisan> adeuring: thanking you :)
[16:26] <barry> adeuring: when you raise NotImplementedError in your properties, you don't need to instantiate them if they have no arguments.  Python will DTRT for you here, and it's also somewhat more efficient
[16:27] <adeuring> barry: Ah, thanks! didn't know that
[16:27] <adeuring> (Though I hope the excpetion will never be raised ;)
[16:28] <barry> adeuring: if the exception has arguments, instantiating it in place is the right thing to do though.  
[16:28] <barry> adeuring: right! :)
[16:32] <adeuring> al-maisan: r=me
[16:33] <al-maisan> adeuring: thank you very much!
[16:33] <adeuring> al-maisan: immer gerne :)
[16:33] <al-maisan> adeuring: :)
[16:38] <barry> adeuring: with that fix, r=me
[16:38] <adeuring> barry: thanks!
[16:40] <barry> rockstar: ping
[16:40] <rockstar> barry, yo
[16:41] <barry> rockstar: hiya!  could you do a quick second ui review on https://code.edge.launchpad.net/~barry/launchpad/440220-slot/+merge/12786
[16:41] <rockstar> barry, sure.
[16:41] <barry> rockstar: thanks!
[16:41] <mrevell> barry: Can you take a look at small help branch? https://code.edge.launchpad.net/~matthew.revell/launchpad/bug-215798/+merge/12792
[16:42] <barry> mrevell: sure thing
[16:42] <mrevell> thanks barry
[16:43] <barry> mrevell: did you forget to bzr add register-branch.html perhaps?  it's not in the diff
[16:43]  * mrevell hangs head in shame
[16:44] <mrevell> barry: Yup. Sorry. Pushing new version now
[16:44] <mrevell> barry: done
[16:44] <barry> mrevell: can you add the file as a comment.  i don't think the mp will update the diff
[16:44] <rockstar> mrevell, I have bzr ci aliased to `bzr commit --strict` and I use that all the time.  It makes sure you don't have any rogue files laying around.
[16:44] <barry> rockstar: +1
[16:44] <rockstar> barry, it will update the diff.
[16:44] <mrevell> barry: I thought we had moving diffs now
[16:44] <mrevell> rockstar: thanks for the top!
[16:45] <rockstar> barry, it updates the diff on push of the source branch now.
[16:45] <barry> rockstar: \o/
[16:45] <mrevell> s/top/tip
[16:45] <rockstar> barry, abentley is pretty amazazing.
[16:45] <barry> rockstar: he's amazing, too!
[16:46] <mrevell> :)
[16:46] <rockstar> barry, I was using a word from a movie, so the apparent typo was actually purposeful.
[16:46] <barry> rockstar: oh!  what movie?
[16:48] <rockstar> barry, it's from "The Benchwarmers"  The dialogue goes something like "You guys think you're ath-a-letes now?"  "I didn't know athletes had three syllables.  That's amazazing!"
[16:48] <barry> mrevell: r=me
[16:48] <mrevell> thanks barry
[16:48] <barry> rockstar: guess i have to put that on my netflix now :)
[16:49] <rockstar> barry, it's appropriate for Max too.
[16:52] <barry> rockstar: great review quotes: "filled with sprays of vomit, fountains of spit and enough hot body air to launch a flotilla of passenger balloons" and "a movie packed with so many idiot characters that rob schneider is cast as the cool guy -- and sort of pulls it off"
[16:52]  * barry clicks add to queue :)
[16:52] <rockstar> barry, :)
[21:58] <barry> EdwinGrubbs, rockstar ping
[21:58] <EdwinGrubbs> barry: pong
[21:59] <barry> hi EdwinGrubbs can you take a quick look at a testfix diff for me?
[21:59] <barry> EdwinGrubbs: http://pastebin.ubuntu.com/284096/
[22:02] <EdwinGrubbs> barry: is the facetmenu class have any stylesheets, or is it just a marker?
[22:02] <barry> EdwinGrubbs: it has some css, but base-layout.txt doesn't test it.  it is a horrible horrible test, so this just repairs it in the easiest possible way
[22:05] <EdwinGrubbs> barry: r=me
[22:06] <barry> EdwinGrubbs: thanks!  testfix here i come