=== 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 | ||
thumper | rockstar: https://code.edge.launchpad.net/~thumper/launchpad/new-code-import-owner/+merge/23974 | 02:28 |
---|---|---|
=== 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 | ||
rockstar | thumper, https://code.edge.launchpad.net/~rockstar/launchpad/recipe-index-redux/+merge/23976 | 03:12 |
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. | 04:39 |
=== jtv1 is now known as jtv | ||
noodles775 | adiroiban: do you want me to land your bug-487137 now? | 07:19 |
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:22 |
adiroiban | noodles775: yes. they need to be tested | 07:23 |
adiroiban | noodles775: can you please also send this branch https://code.edge.launchpad.net/~adiroiban/launchpad/bug-201749/+merge/21250 ? | 07:24 |
noodles775 | Sure. | 07:24 |
=== 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 | ||
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. | 10:20 | |
=== 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 | ||
bigjools | adeuring: hi, you seem to have removed my branch from the queue, unless you reviewed it? | 11:09 |
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:10 |
bigjools | adeuring: ah ok, dunno what reset the topic then, it was there earlier! | 11:11 |
bigjools | thanks | 11:11 |
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:15 |
noodles775 | yeah, I was surprised too. | 11:16 |
adeuring | noodles775: r=me, a few nitpicks | 11:22 |
wgrant | 5 items were mysteriously dropped from the queue 9 hours ago. | 11:23 |
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 | 11:24 |
=== 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 | ||
noodles775 | wgrant: Thanks! Could you pop them back in so other people don't get annoyed? :) | 11:25 |
=== 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 | ||
bigjools | adeuring: yes, thanks | 11:38 |
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:48 |
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:49 |
bigjools | but happy to be persuaded otherwise? | 11:50 |
adeuring | noodles775: sorry... stupid copy&paste mistake. I've added a comment | 11:50 |
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:51 |
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:52 |
adeuring | bigjools: so, r=me | 11:53 |
bigjools | adeuring: great, thanks very much | 11:53 |
=== 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 | ||
adeuring | noodles775: r=me | 12:31 |
=== 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 | ||
noodles775 | Thanks adeuring | 12:54 |
=== 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 | ||
* adeuring does not do ui reviews... | 13:21 | |
=== 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 | ||
sinzui | adeuring, thanks for seeing the bug I added to the code. I will reply with a fix soon | 14:49 |
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:29 |
sinzui | adeuring, I replied to your review | 15:34 |
* adeuring is looking | 15:34 | |
adeuring | sinzui: approved | 15:39 |
sinzui | adeuring, thanks | 15:39 |
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:47 |
noodles775 | Wow... the re-org. looks excellent. | 15:48 |
sinzui | noodles775, it was defined twice! | 15:48 |
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:50 |
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:52 |
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:54 |
noodles775 | sinzui: not for a few years, I'll give it a whirl next time I do some css work. | 15:57 |
noodles775 | sinzui: another (build) tool I've used in the past for nicer stylesheet organisation is http://sass-lang.com/ | 15:58 |
sinzui | that looks practical and is compatible with mars' ideas | 15:59 |
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:00 |
noodles775 | lol | 16:01 |
sinzui | It wanted to be perl done right, but soon learned python done right is even better | 16:01 |
sinzui | I like ruby's symbol feature. I hope python steals that idea | 16:02 |
noodles775 | henninge: done. | 16:04 |
henninge | cheers | 16:04 |
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:12 |
mrevell | Thanks noodles775, I'll leave it until after the weekend in that case. | 16:13 |
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:14 |
mrevell | sinzui, https://code.edge.launchpad.net/~matthew.revell/launchpad/bug-heat-help-bug-544799/+merge/24016 | 16:15 |
=== 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 | ||
sinzui | mrevell, you may cry. my branch is an extensive reorg. You will may want to update/add the styles by hand. | 16:16 |
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 :) | 16:17 |
=== 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 | ||
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:34 |
* sinzui looks | 19:35 | |
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:36 |
bac | bah | 19:37 |
bac | thanks, though | 19:37 |
=== gary-lunch is now known as gary_poster | ||
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 | 20:45 |
=== EdwinGrubbs is now known as Edwin-lunch | ||
sinzui | oh, I have reviewed the build_doctest_suite before. I glad the work will see the light of day | 21:02 |
sinzui | bac: I cannot review your proposal. It is read-only | 21:05 |
bac | well that's dumb | 21:05 |
sinzui | bac: you are listed as the only person who can review it | 21:06 |
bac | weird | 21:06 |
bac | sinzui: and now? | 21:06 |
bac | sinzui: i tried to make it depend on my previous branch but it wants to merge into it | 21:07 |
bac | let me blow away that MP and try again | 21:08 |
sinzui | oh | 21:09 |
sinzui | I was just reviewing it | 21:09 |
sinzui | bac: sorry you had fixed it | 21:09 |
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:10 |
* sinzui reviews the next proposal before a vengeful bac kills it | 21:11 | |
bac | i think the first one was doomed | 21:11 |
sinzui | bac: I want to rename your kanban card to be about refactoring lplib tests. | 21:20 |
bac | ok | 21:20 |
sinzui | bac: is your branch done with my review? Can I move the card to Review/Done? | 21:21 |
bac | yes | 21:21 |
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 | 21:25 |
=== Edwin-lunch is now known as EdwinGrubbs | ||
* sinzui claims it before bac deletes it | 21:25 | |
EdwinGrubbs | sinzui: btw, I blame pipeline for the completely unnecessary prereq branch. | 21:26 |
sinzui | EdwinGrubbs, I see you found an elegant fix in the factory | 21:28 |
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:33 |
EdwinGrubbs | sinzui: do you want me to add a test to doc/account? | 21:34 |
sinzui | no. I think there is a unit test for it. From launchpad's perspective, we care about the person. | 21:35 |
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:36 |
sinzui | EdwinGrubbs, I approved your branch. | 21:37 |
EdwinGrubbs | sinzui: thanks | 21:38 |
=== matsubara is now known as matsubara-afk |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!