[00:34] <lifeless> jml: your oops has come up : https://lp-oops.canonical.com/oops.py/?oopsid=OOPS-1719ED569
[00:34] <lifeless> jml: that doesn't look like you to me.
=== EdwinGrubbs changed the topic of #launchpad-reviews to: On call: - || Reviewing: jelmer || queue: [allenap, lifeless]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[02:25] <EdwinGrubbs> lifeless: If you have time, I have a branch that needs a db review. https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-615654-registry-jobqueue-schema/+merge/35619
[02:27] <StevenK> EdwinGrubbs: Hi! Can you please look at the MP you reviewed for me last night?
[02:28] <StevenK> EdwinGrubbs: I agree with your changes, I've applied and tested them.
[02:28] <lifeless> EdwinGrubbs: hi, stub needs to review it
[02:29] <lifeless> EdwinGrubbs: the policy is 'request from both of us so we both know' and 'stub does it unless hes on leave'
[02:29] <EdwinGrubbs> StevenK: I'll look at it now.
[02:32] <EdwinGrubbs> StevenK: that looks good. r=me
[02:32] <StevenK> EdwinGrubbs: Thanks!
[02:33] <EdwinGrubbs> lifeless: ok, thanks
[02:34] <lifeless> EdwinGrubbs: you did raise an interesting question though and I've commented there
[02:34] <lifeless> EdwinGrubbs: I was talking with deryck this morning about having a single deferred email dispatcher
[02:35] <lifeless> that would jsonify-or-so *all* the emails an appserver wants to send, shove that into one job, and then it gets committed as part of the transaction
[02:35] <lifeless> and a Job would process those for the whole system after
[02:41] <StevenK> Someone around who can review a small branch?
[02:41] <StevenK> 1 small code change, mostly just lint
[02:44] <lifeless> yes
[02:45] <StevenK> lifeless: https://code.edge.launchpad.net/~stevenk/launchpad/db-clean-up-lint-idsjob/+merge/35621
[03:25] <StevenK> lifeless: Sorry to do this, but I'm about to head out, and I'd like that branch on ec2 while I'm out, can you push it up your stack, please?
[03:32] <lifeless> StevenK: I did it already
[03:36] <StevenK> lifeless: Sorry, I'm a muppet.
[03:36] <lifeless> yes
[03:36] <lifeless> :P
=== StevenK changed the topic of #launchpad-reviews to: On call: - || Reviewing: jelmer || queue: [allenap, lifeless, StevenK]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || Reviewing: jelmer || queue: [allenap, lifeless, StevenK]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[10:12] <jelmer> resolved 7 conflicted files.. posting the diff now
[10:13] <noodles775> Thanks jelmer
=== noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || Reviewing: jelmer || queue: [allenap, lifeless, StevenK, noodles775]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[10:58] <noodles775> henninge: I've got another MP needing UI thought if you've time (but no rush): https://code.edge.launchpad.net/~michael.nelson/launchpad/635005-difference-details-2/+merge/35640
[10:59] <henninge> noodles775: wow, with screencast?
[11:00] <henninge> noodles775: you rock!
[11:00] <noodles775> henninge: well, I try to make it easy to understand (hard to describe in text) :)
[11:01] <henninge> noodles775: really great! Didn't you describe somewhere how you do these? Or did someone else?
[11:02] <noodles775> henninge: I did put something on the wiki at some point... it's just gtk-recordmydesktop, thank them :)
[11:03] <henninge> noodles775: I would not expect to have a standard for comments with logos because we have never had any of those.
[11:04] <henninge> noodles775: on first view they struck me as taking up a lot of space.
[11:04] <henninge> noodles775: my take: we should not have two types of comments "with" and "without" logos.
[11:04] <noodles775> henninge: yeah, I noted that in the MP, saying I should probably just do the same as mp/bug comments. The space is because the person logo (ie. it would be there photo), only comes in the 64x64 size.
[11:05] <noodles775> s/there/their
[11:05]  * henninge did not read ... just watched in awe .. ;)
[11:05]  * henninge reads now
[11:07] <henninge> noodles775: yes, I think the comments should look like other comments we have elsewhere.
[11:07] <henninge> or we should commit to changing all comments to include headshots ... ;)
[11:07] <noodles775> yep, agreed.
[11:08] <henninge> noodles775: that multiline comment - does it have double new lines or why is the line spacing so big?
[11:09] <noodles775> henninge: the formatter being applied treats double new-lines in text as paragraph boundaries... I think I need to ensure paragraphs have less margin in those comments.
[11:11] <henninge> noodles775: It should be treated by the css for comments, like on any other comment.
[11:11] <henninge> noodles775: Meaning: I don't think you need to worry about that much.
[11:11] <noodles775> henninge: yes, that's what I meant (ie. I need to check...)
[11:12] <noodles775> henninge: are you running maverick?
[11:12] <henninge> yes
[11:13] <noodles775> Can you please just run one test for db-devel and see if you get the error I mention on the MP?
[11:14] <noodles775> I've just tried the following with r9787 of db-devel (which went through buildbot recently), but always get the error linked on the MP.
[11:14] <noodles775> bin/test -vv -m test_archive -t test_getPublications_returns_all_published_publications
[11:14] <noodles775> (just a random test).
=== allenap changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: jelmer, - || queue: [allenap, lifeless, StevenK, noodles775]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[11:14] <noodles775> (well, one that uses the DB).
[11:14] <allenap> Good morning noodles775 :)
[11:14] <noodles775> Hi allenap! Welcome back!
[11:14] <henninge> noodles775: you need to downgrade psycopg
[11:15] <allenap> noodles775: Cheers :) Right, on to lifeless's branch.
=== allenap changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: jelmer, lifeless || queue: [allenap, StevenK, noodles775]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[11:15] <noodles775> henninge: ah, I do remember glancing over something with that info... thanks.
[11:16] <henninge> noodles775: download from here https://edge.launchpad.net/ubuntu/lucid/i386/python-psycopg2/2.0.13-2ubuntu2
[11:16] <henninge> (edge css is broken again)
[11:16] <henninge> noodles775: then do "dpkg -i --force-downgrade" with it.
[11:16] <noodles775> Thanks.
=== allenap changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: jelmer, StevenK || queue: [allenap, noodles775]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[11:21] <allenap> Gah, not doing very well this morning.
[11:21] <noodles775> Oh? I just thought you were super fast :)
[11:21] <allenap> noodles775: Which of jelmer's branches are you reviewing? I'll do the other.
[11:22] <noodles775> allenap: sorry, this one: https://code.edge.launchpad.net/~jelmer/launchpad/506256-remove-popen-2/+merge/35412
[11:22] <noodles775> I've just claimed to avoid more confusion :)
=== allenap changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: jelmer, jelmer || queue: [allenap, noodles775]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[11:23] <henninge> noodles775: I don't understand what the "Pacakge differences" section is telling me.
[11:24] <noodles775> henninge: yes - it may need more explanation. So when there are different versions of a package in the two distro series...
[11:24] <noodles775> there will be a base version (ie. the last common version before they diverged)
[11:25] <noodles775> Those differences will be available on request (or generated automatically - still need to do that), to show the difference between the base version and each distro series version.
[11:26] <henninge> noodles775: Ah, I see. Those will be clickable and expand, too?
[11:26] <noodles775> henninge: no, that wasn't planned, but could be nice.
[11:26] <henninge> noodles775: isn't the "last common version" information missing?
[11:27] <henninge> noodles775: but they will be linkified to an extra page?
[11:27] <noodles775> henninge: yes, it says "unimplemented" in the demo (as I need to add a db column for it).
[11:27]  * henninge is blind
[11:27] <noodles775> And yes, the diffs will link to librarian files.
[11:28] <henninge> noodles775: and it will always be those two link: "base to parent" and "base to derived" ?
[11:30] <noodles775> henninge: sometimes just one (ie. when a package hasn't been updated in the parent series, but only the derived, it's no point generating the base-parent diff)
[11:36] <henninge> noodles775: so, because they are downloads AFAIUI, they should be preceeded by the download logo.
[11:37] <noodles775> They don't work as downloads on other pages... let me check.
[11:37] <noodles775> (not that that means they shouldn't)
[11:38] <henninge> noodles775: links to librarian?
[11:38] <noodles775> links to text files on the librarian.
[11:38] <henninge> that depends on your browser settings, I think
[11:39] <noodles775> henninge: right, and it's a .gz anyway...
[11:39] <noodles775> So currently PackageDiffs aren't marked up as download links, but that would be great to change. +1
[11:40] <henninge> noodles775: maybe call the section "Package differences from base version:"
[11:40] <henninge> noodles775: oh, where do I normally see them?
[11:40]  * henninge has not worked with those before
[11:44] <noodles785> *sigh*
[11:44] <noodles785> Last thing I said was:Expand 'hello' and you'll see a link for an available diff.
[11:45] <noodles785> (for the link https://edge.launchpad.net/~michael.nelson/+archive/pocketsphinx/+packages?field.name_filter=&field.status_filter=&field.series_filter= )
=== noodles785 is now known as noodles775
[11:48] <henninge> noodles775: yes, those should have the download icon, too.
[11:48] <henninge> noodles775: I also wonder if a little indention sound be fitting because it is a list (mostly two entries).
[11:48] <noodles775> jelmer: You've added some delete perms to the uploader (now that you're removing the buildqueue entries earlier), but does that mean we can remove DELETE perms elsewhere?
[11:49] <noodles775> henninge: yes on both counts.
[11:49] <jelmer> noodles775: That's a good point, let me check
[11:49] <henninge> I don't know how "sound" got in there ... ;)
[11:50] <henninge> I probably meant "would" ...
[11:50] <noodles775> Yeah, I assumed you meant 'could' :)
[11:52] <noodles775> jelmer: also, I guess you tried store.flush() before you ended up with the txn.commit()?
[11:52] <noodles775> (or you know why that wouldn't work :) ).
=== allenap changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: jelmer, lunch || queue: [allenap, noodles775]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[11:59] <jelmer> noodles775: Yeah, I did - although not for all instances individually.
[12:05] <henninge> noodles775: reviewed, needs-fixing as discussed.
[12:06] <noodles775> Great, thanks henninge, I'll finish the windmill test too before requesting a re-review.
=== noodles775 changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: lunch, lunch || queue: [allenap, noodles775]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[12:24] <noodles775> allenap: did you review StevenK's branch? I can't see one on the active reviews page either, but I did think he mentioned he had one. StevenK ?
[12:25] <noodles775> jelmer: review sent... great work, just a few questions.
[12:27] <jelmer> noodles775: Thanks
[12:29] <StevenK> noodles775: Sorry, had the silly thing as WIP
[12:30] <StevenK> noodles775: https://code.edge.launchpad.net/~stevenk/launchpad/db-add-derivedistroseries-api/+merge/35500
=== mrevell is now known as mrevell-lunch
[13:04] <jelmer> allenap: Thanks for the review
=== matsubara-afk is now known as matsubara
[13:16] <allenap> jelmer: You're welcome :)
=== allenap changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: lunch, noodles775 || queue: [allenap]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== allenap changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: lunch, StevenK || queue: [allenap]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== allenap changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: lunch, wallyworld || queue: [allenap]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== noodles775 changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: allenap, wallyworld || queue: []  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== allenap changed the topic of #launchpad-reviews to: On call: noodles775, allenap || Reviewing: allenap, StevenK || queue: []  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[13:42] <noodles775> allenap: what order of subscriptions are returned by getSubscriptionsFromDuplicates()?
[13:43]  * allenap looks
[13:44] <noodles775> sorry, I meant how many subscriptions - and whether it's currently a method with performance issues or not.
[13:46] <allenap> noodles775: I don't know if it has performance problems, but the code certainly appears sub-optimal. It should return one subscription per subscriber. Each subscriber could have more than one subscription so I've ensured that the earliest is always returned. I /think/ it was undefined before.
[13:54] <noodles775> allenap: why is it valid for a person to have multiple (current/valid) subscriptions to the same bug? Or is it just because it can also be via teamparticipation?
[13:55] <noodles775> Actually, looking at that query, it's only subscriptions for the person directly, so why can they have more than one?
[13:55] <allenap> noodles775: It's because the subscriptions are to duplicates. There could be several duplicates for a bug and one person could be subscriber to more than one.
[13:55] <noodles775> Right - just spotted that... sorry for the noise.
[13:58] <allenap> noodles775: It's not noise! :) It's quicker to ask than to pore over code. Although I regularly pore over code when I could ask, ahem.
[13:59] <allenap> noodles775: http://paste.ubuntu.com/494739/ <-- a small optimization to that branch. Do you mind if I smuggle that in?
[13:59] <noodles775> 'course not :)
[14:01] <allenap> noodles775: Thanks :)
=== Ursinha-afk is now known as Ursinha
[14:07] <jelmer> noodles775: I've done the checks you mentioned your the review, and managed to avoid the remoteSecurityProxy call. However, I'd like to make an additional change - flushing the store of build in packagebuild to avoid potential race conditions between buildmaster and the upload processor.
[14:07] <jelmer> Does that seem reasonable?
[14:08] <noodles775> jelmer: sure.
[14:15] <noodles775> allenap: OK, done. I added a thought for possibly doing that query all in the db... but haven't checked to see if it would work.
[14:15] <allenap> noodles775: I'm interested.
[14:18] <noodles775> jelmer: have you/can you push an incremental?
[14:18] <jelmer> noodles775: ok
[14:43] <jelmer> noodles775: updated diff posted
[14:43] <noodles775> Thanks jelmer
=== mrevell-lunch is now known as mrevell
=== benji____ is now known as benji
[14:48] <jelmer> noodles775: I haven't looked at splitting up those two tests yet.
[14:49] <noodles775> jelmer: that's fine - it was just a thought, I didn't expect you to necessarily do it unless you were really keen :)
[14:56] <allenap> noodles775: Thanks for the query suggestion. Does this look sane to you? http://paste.ubuntu.com/494766/
[14:57] <noodles775> allenap: yeah, great stuff! (assuming it works?)
[14:57] <allenap> noodles775: The tests pass ;)
[14:58] <noodles775> Wonderful :)
[14:58] <allenap> noodles775: To be honest, I'm not sure that there's sufficient coverage on this method, so I'm going to add another test.
[14:59] <noodles775> allenap: ensuring that the bug subscription is indeed the first one, or something else?
[15:01] <allenap> noodles775: Yes. I didn't look closely before, but now I find that the only direct test I can find for getSubscriptionsFromDuplicates is right at the end of a doctest, and it doesn't make me feel awfully comfortable.
[15:06] <noodles775> Sounds excellent (and you could even remove the doctest example if it's not relevant documentation-wise?)
[15:09] <noodles775> jelmer: did you mean to paste the whole diff in that comment, or just the incremental? (the MP will update the diff when you push your branch right?). I'm trying to get an incremental but it looks like you haven't pushed your branch? (last rev. is 11106 20hrs ago).
[15:13] <jelmer> noodles775: http://pastebin.ubuntu.com/494772/
[15:17] <noodles775> jelmer: looks fine. Were those tests failing previously? (the one you've updated there that assumed the upload log would be present, but now asserts that it is not)?
[15:17] <jelmer> noodles775: I indented build.storeUploadLog(), which changed the behaviour slightly.
[15:17] <jelmer> noodles775: So we're not storing the upload log when the upload succeeded, consistent with previous behaviour (see the discussin in #launchpad-dev earlier for background)
[15:21] <noodles775> Yeah, I saw parts of it flash by :)
=== noodles775 changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: StevenK || queue: []  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== Ursinha is now known as Ursinha-lunch
=== salgado is now known as salgado-lunch
[16:42] <leonardr> allenap, i have a slightly oversized branch coming towards you
[16:57] <allenap> leonardr: I might not have time to do it today :-/
[16:57] <leonardr> allenap: ok, np
[16:57] <leonardr> take a look once i get the mp up
[16:57] <allenap> Cool.
=== benji is now known as benji-lunch
=== matsubara is now known as matsubara-lunch
=== Ursinha-lunch is now known as Ursinha
[17:38] <bdmurray> Is there somebody I should ping about my UI review for https://code.edge.launchpad.net/~brian-murray/launchpad/limited-subscriptions-page/+merge/35177 ?
=== deryck is now known as deryck[lunch]
=== salgado-lunch is now known as salgado
=== benji-lunch is now known as benji
[18:15] <bdmurray> salgado: is there a queue for ui reviews?
[18:17] <salgado> bdmurray, not that I know of
[18:19] <bdmurray> salgado: Then I just ping a UI reviewer?
=== matsubara-lunch is now known as matsubara
[18:26] <salgado> bdmurray, yep, that's how people have been telling me about ui reviews
[18:26] <bdmurray> salgado: ping ;-) https://code.edge.launchpad.net/~brian-murray/launchpad/limited-subscriptions-page/+merge/35177
[18:32] <salgado> bdmurray, heh, ok, I should be able to review it later today, but can't promise because I need to wrap up other stuff and I'm kinda slow because I've got one arm in a sling
[18:36] <leonardr> well, allenap can't take it, but maybe someone else can look at https://code.edge.launchpad.net/~leonardr/launchpad/accept-oauth-signatures/+merge/35697 ?
[18:36] <leonardr> i'd really like salgado to look at it but i don't want to put more work on his plate
[18:36] <leonardr> bdmurray, is your branch something i can look at?
[18:40] <bdmurray> leonardr: If you can do UI reviews sure.
[18:41] <leonardr> bdmurray: sorry, not rated for ui reviews
[18:43] <bdmurray> salgado: sorry about your arm - its no hurry I just wanted to get in some queue
=== deryck[lunch] is now known as deryck
[19:43] <jelmer> Hi StevenK
[19:48] <jelmer> StevenK: Can I add a simple soyuz branch to your queue?
[19:53] <lifeless> jelmer: I think you mean allenap
[19:54] <jelmer> lifeless: You're right, thanks
[19:54] <jelmer> I think allenap has probably signed off by now though :-(
[19:56] <jelmer> allenap: Still there?
[19:56] <lifeless> jelmer: link the branch earlier ;)
[20:00] <jelmer> lifeless: :-) It's at https://code.edge.launchpad.net/~jelmer/launchpad/635591-sync-source-unicode/+merge/35715
[20:02] <lifeless> jelmer: your description references the same bug, not an older one?
[20:03] <jelmer> lifeless, it's a fix for an issue introduced by the fix for bug 635591, and found while I was QA'ing that.
[20:03] <lifeless> but it appears to be the same branch
[20:03] <lifeless> with everything the same
[20:03] <lifeless> it says its the fix for itself
[20:03] <jelmer> The last revision is new
[20:03] <lifeless> there's only one revision there
[20:04] <lifeless> unmerged that is
[20:04] <jelmer> Yes, the original fix has already landed on devel.
[20:04] <lifeless> so where is the original MP?
[20:05] <lifeless> look at this with fresh eyes
[20:05] <lifeless> where is the qa-bad tagged bug
[20:05] <lifeless> so that we know not to rollout the thing that failed qa
[20:05] <jelmer> https://code.edge.launchpad.net/~jelmer/launchpad/635591-sync-source-unicode/+merge/35637
[20:06] <lifeless> and why isn't that MP linked to the branch anymore ?
[20:06] <lifeless> ah someone rebased.
[20:06] <lifeless> ?
[20:07] <jelmer> I'm not sure why it's no longer linked.
[20:07] <leonardr> sinzui or mars, do you have any interest in doing an interactive review?
[20:08] <jelmer> lifeless: I've se the qa-bad tag.
[20:08] <sinzui> leonardr, I will have time in an hour
[20:08] <lifeless> approved
[20:08] <leonardr> sinzui: ok, you're on
[20:09] <jelmer> lifeless: Thanks
[21:40] <sinzui> leonardr, sorry about my delays. I am ready to review
[21:40] <leonardr> sinzui, great
[21:40] <leonardr> the branch is https://code.edge.launchpad.net/~leonardr/launchpad/accept-oauth-signatures/+merge/35697
[21:40] <leonardr> if there's anything you don't understand about the system i'm changing or what i'm aiming for, tell me andi 'll go through it
[21:43] <sinzui> leonardr, I am ready, the backstory is interesting
[21:44] <leonardr> sinzui: ok, go through the helpers i added to _webservice.py, and then see what i did with them in authorize-token.txt
[21:49] <sinzui> leonardr, do we care if I hack my user-agent string?
[21:51] <leonardr> sinzui: the user-agent thing is mostly to avoid any plausible deniability that an unauthorized client might have "accidentally" acquired someone's GRANT_PERMISSIONS token. it can't provide any real security
[21:55] <sinzui> from canonical.launchpad.interfaces import IPersonSet becomes
[21:55] <sinzui> from lp.registry.interfaces.person import IPersonSet
[21:55] <sinzui> leonardr, ^ c.l.interfaces is deprecated
[21:56] <leonardr> thanks
=== salgado is now known as salgado-afk
[21:56] <sinzui> Though I am certain that IPersonSet will be the very last thing we remove
[21:56] <sinzui> leonardr, I am at 435 and I an very impressed.
[21:58] <sinzui> leonardr, are you using no-priv because this is a well known user? Is there reason to not use a person create with the factory?
[21:58] <leonardr> sinzui: i don't think it matters, i just haven't used the factory much
[22:08] <sinzui> leonardr, I have no other questions. This is a nice implementation and I see you tested the state conditions
[22:09] <leonardr> sinzui, thanks. can you point me to a good example of a pagetest that uses the factory?
[22:10] <sinzui> I think so
[22:11] <sinzui> leonardr, the factory is already in the test name space. so the minimum you need it
[22:12] <sinzui> is user = factory.makePerson(name='test-user')
[22:12] <sinzui> leonardr, you can also use login_person(user)
[22:13] <sinzui> look in lp/registry/stories/webservice/xx-project-registry to see a lot of factory uses in a story
[22:15] <leonardr> i think i have it...
[23:07] <jelmer> any rc reviewers around?
=== matsubara is now known as matsubara-afk