=== Edwin-lunch is now known as EdwinGrubbs
=== adiroiban changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [adiroiban(UI bug-146178), noodles, bigjools, sinzui, adiroiban(bug-402235)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[02:28] <thumper> rockstar: https://code.edge.launchpad.net/~thumper/launchpad/new-code-import-owner/+merge/23974
=== thumper changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[03:12] <rockstar> thumper, https://code.edge.launchpad.net/~rockstar/launchpad/recipe-index-redux/+merge/23976
[04:39] <abentley> thumper, could you please review https://code.edge.launchpad.net/~abentley/launchpad/build-from-recipe-api/+merge/23981 ?
[04:39] <thumper> were we too noisy?
[04:39] <abentley> thumper, no, my bowels were too full.
=== jtv1 is now known as jtv
[07:19] <noodles775> adiroiban: do you want me to land your bug-487137 now?
[07:22] <adiroiban> noodles775: only if you have time :)
[07:22] <noodles775> adiroiban: heh, it takes all of 10 seconds ;), and the same for 61081 I assume?
[07:22] <noodles775> Thanks for the updates!
[07:23] <adiroiban> noodles775: yes. they need to be tested
[07:24] <adiroiban> noodles775: can you please also send this branch https://code.edge.launchpad.net/~adiroiban/launchpad/bug-201749/+merge/21250 ?
[07:24] <noodles775> Sure.
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[10:20] <noodles775> adeuring: can I get you to take a look at: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-build-farm-job-model/+merge/23913
[10:20] <adeuring> noodles775: sure
[10:20] <noodles775> Thanks!
[10:20]  * noodles775 has another one which will follow too.
=== noodles775 changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [noodles*2] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: noodles || queue: [noodles] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[11:09] <bigjools> adeuring: hi, you seem to have removed my branch from the queue, unless you reviewed it?
[11:10] <adeuring> bigjools:sorry... I think the queue was empty when I joined the channel... I even wasn't aware that you asked for a reveiw...
[11:10] <adeuring> bigjools: will look at your branch when I've finished noodles'
[11:11] <bigjools> adeuring: ah ok, dunno what reset the topic then, it was there earlier!
[11:11] <bigjools> thanks
[11:15] <noodles775> bigjools: when I logged in this morning at 7:00UTC, the queue was empty :/
[11:15] <bigjools> oh :/
[11:15] <bigjools> it was there last night, prob what I am thinking of
[11:16] <noodles775> yeah, I was surprised too.
[11:22] <adeuring> noodles775: r=me, a few nitpicks
[11:23] <wgrant> 5 items were mysteriously dropped from the queue 9 hours ago.
[11:24] <adeuring> bigjools: i assume this is your branch: https://code.edge.launchpad.net/~julian-edwards/launchpad/delete-ppa-part2/+merge/23967 ?
[11:24] <noodles775> Thanks adeuring
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: bigjools || queue: [noodles] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[11:25] <noodles775> wgrant: Thanks! Could you pop them back in so other people don't get annoyed? :)
=== wgrant changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: bigjools || queue: [noodles, adiroiban(UI bug-146178), sinzui, adiroiban(bug-402235)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[11:38] <bigjools> adeuring: yes, thanks
[11:48] <adeuring> bigjools: a naive question: can't you use "if os.path.exists(root_dir)" instead of catching two exceptions?
[11:48] <noodles775> Hi adeuring, I think you forgot to include your comment when you reviewed my branch (instead using the MP description)?
[11:49] <bigjools> adeuring: I prefer the exceptions, something else may go wrong, like permissions being wrong etc.
[11:49] <bigjools> adeuring: it seems more robust
[11:50] <bigjools> but happy to be persuaded otherwise?
[11:50] <adeuring> noodles775: sorry... stupid copy&paste mistake. I've added a comment
[11:51] <adeuring> bigjools: OK. But there may be other errors, like odd permissiono settings preventing the deletion. And now we don't see any specific reason why the deletion failed.
[11:52] <bigjools> adeuring: it prints the reason in the log
[11:52] <adeuring> bigjools: argh, yes, you are right. I did not see the little "e)" at the end of the log.warning() call. Sorry
[11:52]  * adeuring really needs better glasses
[11:52] <bigjools> hehe, np, easily missed
[11:53] <adeuring> bigjools: so, r=me
[11:53] <bigjools> adeuring: great, thanks very much
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: noodles || queue: [adiroiban(UI bug-146178), sinzui, adiroiban(bug-402235)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[12:31] <adeuring> noodles775: r=me
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: [lunch] || queue: [adiroiban(UI bug-146178), sinzui, adiroiban(bug-402235)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[12:54] <noodles775> Thanks adeuring
=== mrevell is now known as mrevellunch
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: adiroiban(UI bug-146178) || queue: [, sinzui, adiroiban(bug-402235)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara-afk is now known as matsubara
[13:21]  * adeuring does not do ui reviews... 
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: sinzui,  || queue: [adiroiban(UI bug-146178), adiroiban(bug-402235)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== mrevellunch is now known as mrevell
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: adiroiban(bug-402235)  || queue: [adiroiban(UI bug-146178)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[14:49] <sinzui> adeuring, thanks for seeing the bug I added to the code. I will reply with a fix soon
[15:29] <sinzui> noodles775, I have an awesome (and I mean that in the historical context) branch that updates CSS. I hope you can find time to review it
[15:29] <noodles775> sinzui: great!
[15:34] <sinzui> adeuring, I replied to your review
[15:34]  * adeuring is looking
[15:39] <adeuring> sinzui: approved
[15:39] <sinzui> adeuring, thanks
[15:47] <noodles775> sinzui: In your MP text, you mention removing the duplicate css for ul/li.language, but afaics MP diff just shows those as moved?
[15:48] <noodles775> Wow... the re-org. looks excellent.
[15:48] <sinzui> noodles775, it was defined twice!
[15:50] <noodles775> sinzui: but on your MP, I can see it added (from a move) on lines 340ff, removed on lines 1578ff, ah, right, then removed again at 3228. OK.
[15:52] <sinzui> noodles775, I really hope that future additions/changes to the css will be easier with this reorganisation. I lost my evening to an obsessive need to understand all the rules we are using
[15:52] <noodles775> sinzui: I think it definitely will... and yes, looking at the organisation I can see why it took an evening!
[15:54] <sinzui> noodles775, have you used the W3C validator? I was looking for a tool in apt and could not find anything as good as the website
[15:54] <henninge> noodles775, jelmer: Can you please re-affirm your review for my branch? I want to merge it into devel.
[15:54] <henninge> https://code.edge.launchpad.net/~henninge/launchpad/bug-565294-devel/+merge/24020
[15:57] <noodles775> sinzui: not for a few years, I'll give it a whirl next time I do some css work.
[15:58] <noodles775> sinzui: another (build) tool I've used in the past for nicer stylesheet organisation is http://sass-lang.com/
[15:59] <sinzui> that looks practical and is compatible with mars' ideas
[16:00] <noodles775> I did mention it a while back, but I think the fact that it's ruby killed the discussion (yet more developer dependencies).
[16:00] <sinzui> ruby needs another 5 years to evolve into python
[16:01] <noodles775> lol
[16:01] <sinzui> It wanted to be perl done right, but soon learned python done right is even better
[16:02] <sinzui> I like ruby's symbol feature. I hope python steals that idea
[16:04] <noodles775> henninge: done.
[16:04] <henninge> cheers
[16:12] <noodles775> mrevell: just a heads-up that I assume your branch will conflict with sinzui's. You may want to wait until his lands and then remerge devel.
[16:13] <mrevell> Thanks noodles775, I'll leave it until after the weekend in that case.
[16:14] <sinzui> mrevell, is your branch styles for help?
[16:14] <mrevell> sinzui, Yes
[16:14] <sinzui> mrevell, you get your own section now
[16:14] <mrevell> sinzui, or, rather, it updates the help style help
[16:15] <mrevell> sinzui, https://code.edge.launchpad.net/~matthew.revell/launchpad/bug-heat-help-bug-544799/+merge/24016
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: -  || queue: [adiroiban(UI bug-146178)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[16:16] <sinzui> mrevell, you may cry. my branch is an extensive reorg. You will may want to update/add the styles by hand.
[16:17] <noodles775> It should just be adding the one line for a.help.icon.
[16:17] <mrevell> sinzui, Heh, no tears here :) As noodles775 says, it was just a one line change.
[16:17] <sinzui> mrevell, you certainly wont have a problem finding your classes in the css.
[16:17] <noodles775> :)
[16:17] <mrevell> Cool :)
=== matsubara is now known as matsubara-lunch
=== adeuring changed the topic of #launchpad-reviews to: On call: - || reviewing: -  || queue: [adiroiban(UI bug-146178)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara-lunch is now known as matsubara
=== gary_poster is now known as gary-lunch
[19:34] <bac> hi sinzui, can you take a look at this small chunk i separated out:  https://code.edge.launchpad.net/~bac/launchpad/bug-569101/+merge/24034
[19:35]  * sinzui looks
[19:36] <bac> sinzui: oops: s/The Launchpad/The Launchpad object/
[19:36] <sinzui> Too late. I just approved it
[19:36] <sinzui> now you have to land it as it is :)
[19:37] <bac> bah
[19:37] <bac> thanks, though
=== gary-lunch is now known as gary_poster
[20:45] <bac> sinzui: have time for another gem?  https://code.edge.launchpad.net/~bac/launchpad/lplib-testing/+merge/24038
[20:45] <bac> a melange, if you will
[20:45] <bac> a cornucopia of code snippets
=== EdwinGrubbs is now known as Edwin-lunch
[21:02] <sinzui> oh, I have reviewed the build_doctest_suite before. I glad the work will see the light of day
[21:05] <sinzui> bac: I cannot review your proposal. It is read-only
[21:05] <bac> well that's dumb
[21:06] <sinzui> bac: you are listed as the only person who can review it
[21:06] <bac> weird
[21:06] <bac> sinzui: and now?
[21:07] <bac> sinzui: i tried to make it depend on my previous branch but it wants to merge into it
[21:08] <bac> let me blow away that MP and try again
[21:09] <sinzui> oh
[21:09] <sinzui> I was just reviewing it
[21:09] <sinzui> bac: sorry you had fixed it
[21:10] <sinzui> bac: my only remark about the code was that I think this line is missing punctuation: # Add doctests using default setup/teardown
[21:10] <bac> sinzui: sorry, can you try again with https://code.edge.launchpad.net/~bac/launchpad/lplib-testing/+merge/24040
[21:11]  * sinzui reviews the next proposal before a vengeful bac kills it
[21:11] <bac> i think the first one was doomed
[21:20] <sinzui> bac: I want to rename your kanban card  to be about refactoring lplib tests.
[21:20] <bac> ok
[21:21] <sinzui> bac: is your branch done with my review? Can I move the card to Review/Done?
[21:21] <bac> yes
[21:25] <Edwin-lunch> sinzui, bac: do either one of you want to review my branch. After I took out an assertion, the size of the branch collapsed. https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-248518-setPreferredEmail/+merge/24041
[21:25] <sinzui> I will
=== Edwin-lunch is now known as EdwinGrubbs
[21:25]  * sinzui claims it before bac deletes it
[21:26] <EdwinGrubbs> sinzui: btw, I blame pipeline for the completely unnecessary prereq branch.
[21:28] <sinzui> EdwinGrubbs, I see you found an elegant fix in the factory
[21:33] <sinzui> EdwinGrubbs, I am surprised there is not comparable test for account.activate() in doc/account.
[21:33] <sinzui> I think your test is the only documentation we have
[21:34] <EdwinGrubbs> sinzui: do you want me to add a test to doc/account?
[21:35] <sinzui> no. I think there is a unit test for it. From launchpad's perspective, we care about the person.
[21:36] <sinzui> EdwinGrubbs, My initial though was to move your test, but I think your decision was right. I think your tests tells other launchpad developer how to get a person's account activated
[21:37] <sinzui> EdwinGrubbs, I approved your branch.
[21:38] <EdwinGrubbs> sinzui: thanks
=== matsubara is now known as matsubara-afk