=== Ursinha is now known as Ursinha-afk === matsubara is now known as matsubara-afk === salgado is now known as salgado-lunch [12:52] hmmm, it seems no one is on call reviewer today. [12:53] adeuring, could I beg a review of you? [12:54] warning that it's a long diff but a simple change -- moving duplicateof to markAsDuplicate. [13:00] deryck: sure [13:00] sorry, was out for lunch [13:01] adeuring, no worries. And thanks! See: https://code.edge.launchpad.net/~deryck/launchpad/do-the-right-thing-dupe-move-78596/+merge/27144 [13:18] deryck: in BugMarkAsDuplicateView.setUpFields(), you set self.form_fields twice. Can't we remove the first assignment? === noodles775 changed the topic of #launchpad-reviews to: On Call: - || reviewing: - || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [13:18] adeuring, let me look at the diff... [13:19] Well-tested trivial review for anyone with a spare few mins (71 lines, mostly tests): https://code.edge.launchpad.net/~michael.nelson/launchpad/589058-upload-rejected-date-started-not-set/+merge/27147 [13:19] deryck: zthatÄs lines 54, 55 of the diff [13:20] adeuring, I cripped this from the methods below where setPrivate is done, for example. I assumed we had to explicitly omit the original field. If not, I can change this. [13:21] cribbed, rather [13:21] deryck: admitteldy, i am not sure... don't know enough about the "field magic"... [13:21] adeuring, I can try it and test. If so, I'll change it. [13:22] sounds good [14:07] deryck: r=me, with 1.5 comments [14:08] adeuring, cool. thanks! === salgado_ is now known as salgado === henninge_ is now known as henninge === gmb changed the topic of #launchpad-reviews to: On Call: - || reviewing: - || queue: [noodles775, gmb(http://bit.ly/bu2arK)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews === EdwinGrubbs changed the topic of #launchpad-reviews to: On Call: Edwin || reviewing: - || queue: [noodles775, gmb(http://bit.ly/bu2arK)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [14:51] EdwinGrubbs: just for when you're ready, mine's pretty small: https://code.edge.launchpad.net/~michael.nelson/launchpad/589058-upload-rejected-date-started-not-set/+merge/27147 [14:53] I'll start on it soon. === leonardr is now known as leonardr-afk [15:51] jtv: can you please look at this before you go? [15:51] https://code.edge.launchpad.net/~henninge/launchpad/bug-591731-is-published-removal/+merge/27166 [15:51] henninge: looking... [15:51] jtv: it's mostly mechanical [15:51] henninge: get a chance to look at the wiki update? [15:51] jtv: no, will do that now. [15:51] Thanks. [15:53] henninge: hope the drive-bys won't cause any conflicts. [15:53] Or not too many, at any rate. [15:54] jtv: I plan to merge stable (or was it db_stable) after my message sharing branch has landed. [15:54] henninge: instead of a variable called diverged_and_not_current_upstream, it may be easier to read to have one called diverged and another called current_upstream. [15:55] recife was based on db-stable. [15:55] jtv: ah, good idea [15:55] Although of course that's long since been released and become devel, and stable, etc. [15:56] henninge: why do you introduce the imported1 and imported2 variables? Are they used somewhere? If not, they'll just cause lint warnings. [15:57] jtv: they are used in the assignment right after it. [15:57] OK [15:57] the tests were breaking because they were missing. [16:06] henninge: I've approved your MP. [16:06] jtv: thanks ;) [16:06] np === Ursinha is now known as Ursinha-lunch === gmb changed the topic of #launchpad-reviews to: On Call: Edwin || reviewing: - || queue: [noodles775)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [16:23] rockstar, I can has review? https://code.launchpad.net/~abentley/launchpad/config-disable-build-requests/+merge/27172 [16:39] sinzui: my branch for the tal test convenience failed in ec2. The fix I made for it might be objectionable, or possibly improvable. Could you look at the diff again? The only part I personally really think you need to review are lines 109-133 of the diff. https://code.edge.launchpad.net/~gary/launchpad/bug586466/+merge/27174 [16:41] gary_poster, you should use push() and pop() when working with the config [16:41] sinzui: but I don't think I can do that with changes of that variable, can I? [16:42] push() is intended to change one or more variables [16:42] that is how we test instances [16:42] OK, will try. [16:43] EdwinGrubbs, could you please review https://code.launchpad.net/~abentley/launchpad/config-disable-build-requests/+merge/27172 ? [16:43] gary_poster, look at lib/canonical/launchpad/pagetests/basic/demo-and-lpnet.txt, line 88 [16:44] abentley, is it urgent, or can you wait until I finish another review. [16:44] gary_poster, the teardown is on line 141 [16:44] ack, looking, thank you [16:47] EdwinGrubbs, not urgent. === matsubara is now known as matsubara-lunch === mwhudson_ is now known as mwhudson [17:22] sinzui: had to adjust thinsg a bit, but is all for better: https://code.edge.launchpad.net/~gary/launchpad/bug586466/+merge/27174, lines 110-117, 125 and 151 are the pertinent ones. [17:24] noodles775, where is the build.duration attribute defined? I don't see it in BinaryPackageBuild or its superclasses. [17:25] EdwinGrubbs: lp.buildmaster.model.buildfarmjob (it returns None if either of the values are None). [17:25] (it's via delegation rather than inheritance) [17:30] noodles775, r=me [17:30] Thanks EdwinGrubbs === deryck is now known as deryck[lunch] === Ursinha-lunch is now known as Ursinha === leonardr-afk is now known as leonardr === matsubara-lunch is now known as matsubara [18:11] gary_poster, r=me [18:11] thank you sinzui! === deryck[lunch] is now known as deryck [19:38] abentley, maybe, you can't actually get to the sourcepackagerecipe +index page normally, but it seems like you need to disable the link returned by SourcePackageRecipeContextMenu.request_builds [19:38] EdwinGrubbs, why? [19:40] abentley, because, I can click on it to get to the form that causes an oops. From your mp, it sounded like you had already removed any way to navigate to that form. [19:40] abentley, even when build_from_branch.enabled=False, the request_builds link is active. [19:41] EdwinGrubbs, we have disabled any way to navigate to that form. The only way you can get there is by URL hacking. [19:41] EdwinGrubbs, going to the +index page is URL hacking. [19:41] abentley, ok, r=me [19:42] abentley, oh, I didn't see that it has already been reviewed and approved. === EdwinGrubbs changed the topic of #launchpad-reviews to: On Call: Edwin || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [20:05] Hi EdwinGrubbs. I have a branch for review I'd like to get on the queue. but I'm EODing soon. [20:05] EdwinGrubbs, can we start now and finish offline, or would you rather I ping someone else later? [20:27] EdwinGrubbs, never mind (for when you're available again). I'll worry about review later. Thanks, anyway. [21:56] rockstar, https://code.launchpad.net/~abentley/launchpad/recipe-build-email/+merge/27199 === matsubara is now known as matsubara-afk [23:44] abentley: Why not change BinaryPackageBuild.notify() to return early when it's FULLYBUILT, rather than adding a special _handleStatus_OK? lp.buildmaster should be generically useful, and not require hacks like that. === EdwinGrubbs 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