=== sinzui changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: leonardr || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [00:20] rockstar: if you are still reviewing: https://code.launchpad.net/~sinzui/launchpad/something-and-nothing/+merge/20191 === rockstar 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 [01:26] sinzui, looking === mup_ is now known as mup === jelmer changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [al-maisan/jelmer] || This channel is logged: http://irclogs.ubuntu.com || === 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 === jtv is now known as jtv-afk [11:07] Good morning adeuring, could you please take a look at https://code.edge.launchpad.net/~al-maisan/launchpad/restricted-ui/+merge/20145 ? [11:07] hi al-maisansure, I'll look [11:07] the diff is not quite right, this is all the branch does: http://pastebin.ubuntu.com/384297/ [11:08] adeuring: ^^ [11:08] sl-thanks! === bigjools changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [11:12] adeuring: simple branch for ya [11:12] bigjools: sure; i just started a review for al-maisan, after that one? [11:12] np [11:15] do we need a database review for a branch that modifies database/schema/security.cfg ? [11:23] al-maisan: r=me; 1 nitpick [11:23] adeuring: thanks :) [11:24] adeuring: I'll take care of the nitpick [11:34] bigjools: r=me [11:35] adeuring: thanks very much [11:35] no nitpicks? :) [11:38] Hi adeuring, I've got an 820 line pure test refactoring branch needing review, but if you'd prefer, I can move the last change out and into the next pipe: https://code.edge.launchpad.net/~michael.nelson/launchpad/ppa-privatisation-test-refactor5/+merge/20207 [11:38] hi adeuring [11:38] adeuring: When you have a chance, can you have a look at https://code.edge.launchpad.net/~jelmer/launchpad/bug471148-ui/+merge/20208 ? [11:38] Hmm, today seems to be soyuz review day :-) [11:39] we're productive dudes [11:39] noodles775: na... that's fine === noodles775 changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [bigjools, noodles775, jelmer] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [11:39] jelmer: sure, i can look. [11:39] Thanks adeuring ! [11:41] adeuring: Thanks [11:41] adeuring: Okay if I add another branch to the queue? === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: noodles775, || queue [jelmer] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === gmb changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [jelmer, gmb] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === gmb changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: noodles775 || queue [jelmer, gmb] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [11:42] gmb: sure, but I just strted a somewhat longer review for noodles775 [11:42] adeuring: Okay, I'll see if I can find someone else to reviw my branch. [12:46] gmb, I'll start my shift shortly and I'll have plenty of time to review yours === matsubara-afk is now known as matsubara [13:02] salgado: allenap's kindly agreed to review it in the meantime; sorry, I forgot to remove it from the queue. Thanks anyway. === gmb changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: noodles775 || queue [jelmer] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [13:05] no worries === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: jelmer || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [13:28] Thanks adeuring . === bigjools changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: jelmer || queue [bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [13:38] adeuring: I have a further tweak to the change you just reviewed, it's on the queue. [13:38] bigjools: ok [13:38] I have to run out to give a presentation on FLOSS to my son's school now! [13:44] bigjools: run, julian, run! === bigjools is now known as bigjools-lunch [13:46] jelmer: could you add doc strings to _get_arm_builds_enabled() and _get_arm_builds_disabled()? [13:47] jelmer: sorry... i mean _set_arm_builds_enabled() instead of the "disabled" [13:47] adeuring: ok [13:49] jelmer: there is an "elif" in set_arms_builds_enabled() without a final "else". Could you add such an "else: pass", together with a comment why nothing needs to be done in this case? [13:51] jelmer: and (sorry about all these nitpick for one method...): The comment "a link is required but it is present" is a bit enigmatic, at least for me. What about sonething like "Arms builds are already enabled"? [13:51] adeuring: Sure, that's more sensible indeed. [13:56] jelmer: getPubSource() in test_publishing.py needs a doc string [14:00] jelmer: other than these nitpicks, r=me [14:00] adeuring: Thanks! [14:01] jelmer: welcome === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: bigjools || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === fjlacoste is now known as flacoste === 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 === al-maisan_ is now known as al-maisan === bigjools-lunch is now known as bigjools [14:59] Hi again adeuring ! I've got the final test refactoring branch ready if you've time: https://code.edge.launchpad.net/~michael.nelson/launchpad/ppa-privatisation-test-refactor6/+merge/20227 [14:59] noodles775: could you ask salgado? I have a somewhat urgent branch myself... [14:59] Sure, salgado? === salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [15:01] noodles775, I'm working on a critical issue, but if gary_poster can't take it I will [15:02] noodles775, salgado: looking === gary_poster changed the topic of #launchpad-reviews to: on call: adeuring,gary_poster || reviewing: ?,noodles775 || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [15:06] salgado: changed topic taking you off reviewers === jtv-afk is now known as jtv [15:06] thanks gary_poster [15:06] noodles775: why did you remove test_login.py? [15:07] gary_poster: I didn't... a local diff between this pipe and the prev shows everything but the test_login.py. [15:07] I was just trying to figure out what was going on with the MP. [15:08] noodles775: ok, I'll just clarify in the review that merging that removal is not ok, and leave you to handle the mechanism :-) [15:11] gary_poster: I'll merge RF into the initial pipe (there are 7 in total) and push it through and see if it's still there when the diff regenerates on the MP. [15:12] noodles775: ack, ok [15:14] gary_poster, noodles775, in my openid branch I added a test_new_login.py file and later removed test_login.py. then on a subsequent branch I renamed test_new_login.py to test_login.py. that, together with pipes, might have confused bzr? [15:15] salgado: thanks for the info - I thought it may have been related to your branch I reviewed yesterday. If that's the case, pumping a fresh RF through my pipes should resolve the issue. (I hope) [15:15] noodles775: in xx-private-builds.txt, why is it OK to remove diff line 375ff? ("Now visit the same page as an admin we can see that there are four more...") [15:15] thanks salgado [15:17] noodles775: also, in the previous section of the same file, why did the result change (5 of 17 results, and different values)? If explaining it takes too long, I'll accept "because of changes that have previously passed review" [15:18] but I'd like reassurance that you are confident of the change, at least :-) [15:19] hm, I could understand an increase of 1, since we added the private p3a archive, but I don't understand 14 -> 17 === jtv is now known as jtv-afk [15:24] sorry gary_poster, I'll be there in a minute or two. [15:24] np noodles775 [15:30] gary_poster: You already have looked at https://code.edge.launchpad.net/~stub/launchpad/memcache/+merge/20226 earlier. Its finished now. [15:31] gary_poster: so, regarding your first question, 375ff, the test for authorised viewers seeing the builds in the history was moved to just below with the Frog builder. [15:31] It looks like a lot less test-code because the Frog builder only has the one build in its history (it's not polluted with sample data :)). [15:31] (it won't land this cycle though if you need to prioritize) [15:32] noodles775: ah ok cool thanks [15:32] gary_poster: and your second question is related, because previously cprov's ppa was switched to private (which is no longer allowed), and had a bunch of publishings. [15:33] stub: cool, I'll take it, unless flacoste, who was just complimenting the branch, tells me that he'll take it. But I'll shepherd it one way or the other [15:33] So the 14 -> 17 is because there are 3 packages in cprov's public archive (when it was private ,they did not show up). [15:33] stub: yeah, I'll take it :-) [15:33] stub: it's not a review, but heh, nice work! i like the syntax a lot! [15:33] ta :) === sinzui changed the topic of #launchpad-reviews to: on call: adeuring,gary_poster || reviewing: ?,noodles775, sinzui || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [15:34] s/packages/builds of course. [15:38] noodles775: we dupe the private setup in xx-source-package-publishing.txt and xx-binary-package-publishing.txt (">>> from lp.registry.interfaces.distribution import IDistributionSet..."). You considered and rejected a helper function? Two copies is arguable, maybe. Three, much less so. :-) [15:39] So if you considered and rejected then I'm OK, but otherwise, please consider :-) [15:41] bac: EdwinGrubbs: are either of you available to review https://code.launchpad.net/~sinzui/launchpad/timeout-portlet-package-summary-2/+merge/20228 [15:41] gary_poster: no I didn't consider it - sounds good. [15:41] ok cool noodles775 [15:42] sinzui: i can do it later. i just made a break through on my testing so i'd prefer not to interrupt that effort ATM [15:42] rock [15:43] adeuring, gary_poster: Are either of you close to be able to review another branch? If not I'll see if I can get a quick review elsewhere. [15:44] allenap: I'd be able to do it in about...30-45 min, I'd guess [15:46] gary_poster: Okay, I'll go and prod someone else. (I don't have much time left to work today, that's the only reason.) Thank you though :) [15:46] allenap: cool, np :-) [15:46] noodles775: I'm finishing the review, but merge-conditional on not landing the deletion of test_login.py [15:47] allenap: I can review it in a few miuntes [15:47] I mean, I'm writing it up [15:47] but it is r=gary with that [15:47] adeuring: deryck is doing it now, but thank you anyway. [15:47] gary_poster: thanks. Actually, if you re-load the MP you'll see that it's no longer there (pumping a fresh RF through seems to have helped) [15:48] noodles775: confirmed, awesome :-) === matsubara is now known as matsubara-lunch === salgado is now known as salgado-lunch === jtv-afk is now known as jtv [16:49] Hi gary_poster, would you object if I land that earlier branch without creating the pagetest helper? I need to leave 20mins ago and would like to send it off to ec2. [16:51] noodles775: that's fine [16:51] gary_poster: ta. === matsubara-lunch is now known as matsubara === salgado-lunch is now known as salgado === deryck is now known as deryck[lunch] === danilo__ is now known as daniloff === deryck[lunch] is now known as deryck === sinzui changed the topic of #launchpad-reviews to: on call: adeuring,gary_poster || reviewing: ?,noodles775 || queue [sinzui] || 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: gary_poster || reviewing: noodles775 || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [18:24] bac: EdwinGrubbs: can either of you review my branch: https://code.launchpad.net/~sinzui/launchpad/timeout-portlet-package-summary-2/+merge/20228 If we cannot land it, we need to RC and remove code next week [18:24] sinzui: yes [18:35] sinzui: is there a bug to put the 'needs linking' section back in? [18:35] assuming we can solve the query timeouts [18:39] no there is not [18:39] bac: I assume we need a model change and job to do daily updates [18:40] sinzui: am i right in assuming we want to restore it? i just don't want to rip it out and then forget about it. [18:41] bac: yes we should restore it after the next two blueprints [18:41] I can report a bug and add an XXX [18:41] cool [18:41] done [19:21] bac: I was able to restore the template. I changed view/needs_linking to return None and left an XXX. We can re-enable the template by uncommenting a line [19:22] ah, sweet === jtv is now known as jtv-eat === gary_poster is now known as gary-lunch [20:55] can anyone take a small branch for review? [20:56] i see that there's a bit of a queue, and it's getting late for me, so if anyone other than gary wants to snatch it now i'll be grateful [20:56] only 115 lines [20:57] rockstar: how about you? you're being suspiciously quiet [20:58] EdwinGrubbs: maybe you fancy a short review? [20:59] intellectronica: I should able to do a review in about 30 minutes. [20:59] EdwinGrubbs: wow, you're amazing. thanks so much. i'm creating an MP now [21:04] EdwinGrubbs: https://code.edge.launchpad.net/~intellectronica/launchpad/heat-decay-complete/+merge/20250 [21:14] sinzui: do you have a branch you need me to look at? === gary-lunch is now known as gary_poster [21:14] gary, no, sorry, bac reviewed it for me === sinzui changed the topic of #launchpad-reviews to: on call: gary_poster || reviewing: noodles775 || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [21:14] cool === gary_poster changed the topic of #launchpad-reviews to: on call: gary_poster || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [21:15] gary_poster: you ditched us last night! [21:16] bac: :-P sorry. talked later with chris rossi at the office and then decided I had had my "drink". I'll stay later next time...esp. when it is closer to home. :-) [21:16] bac, do you thnk you will do any of the hack nights? [21:17] yes, probably. especially if i make any progress on my side project, which hasn't gotten off the ground yet === jtv-eat is now known as jtv [21:45] intellectronica: review sent [21:46] EdwinGrubbs: great, thanks a million! [21:49] intellectronica: so I've figured out what was wrong with export bug linked branches branch [21:49] bdmurray: oh yeah? what was it? [21:50] intellectronica: I believe we were importing and using the wrong thing. But now I'm uncertain how to proceed with a new or modified merge proposal [21:51] bdmurray: excellent. i'm just about to sign off, but i'll be back tomorrow, so if you have something ready i can review and land it for you [21:51] intellectronica: okay, cool. Should I just make a whole new merge proposal against devel and reference the old one? [21:51] bdmurray: sure, that works === matsubara is now known as matsubara-afk === gary_poster 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 === jamalta is now known as jamalta-afk