[12:52] <deryck> hmmm, it seems no one is on call reviewer today.
[12:53] <deryck> adeuring, could I beg a review of you?
[12:54] <deryck> warning that it's a long diff but a simple change -- moving duplicateof to markAsDuplicate.
[13:00] <adeuring> deryck: sure
[13:00] <adeuring> sorry, was out for lunch
[13:01] <deryck> adeuring, no worries.  And thanks!  See:  https://code.edge.launchpad.net/~deryck/launchpad/do-the-right-thing-dupe-move-78596/+merge/27144
[13:18] <adeuring> deryck: in BugMarkAsDuplicateView.setUpFields(), you set self.form_fields twice. Can't we remove the first assignment?
[13:18] <deryck> adeuring, let me look at the diff...
[13:19] <noodles775> 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] <adeuring> deryck: zthatÄs lines 54, 55 of the diff
[13:20] <deryck> 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] <deryck> cribbed, rather
[13:21] <adeuring> deryck: admitteldy, i am not sure... don't know enough about the "field magic"...
[13:21] <deryck> adeuring, I can try it and test.  If so, I'll change it.
[13:22] <adeuring> sounds good
[14:07] <adeuring> deryck: r=me, with 1.5 comments
[14:08] <deryck> adeuring, cool.  thanks!
[14:51] <noodles775> 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] <EdwinGrubbs> I'll start on it soon.
[15:51] <henninge> jtv: can you please look at this before you go?
[15:51] <henninge> https://code.edge.launchpad.net/~henninge/launchpad/bug-591731-is-published-removal/+merge/27166
[15:51] <jtv> henninge: looking...
[15:51] <henninge> jtv: it's mostly mechanical
[15:51] <jtv> henninge: get a chance to look at the wiki update?
[15:51] <henninge> jtv: no, will do that now.
[15:51] <jtv> Thanks.
[15:53] <jtv> henninge: hope the drive-bys won't cause any conflicts.
[15:53] <jtv> Or not too many, at any rate.
[15:54] <henninge> jtv: I plan to merge stable (or was it db_stable) after my message sharing branch has landed.
[15:54] <jtv> 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] <jtv> recife was based on db-stable.
[15:55] <henninge> jtv: ah, good idea
[15:55] <jtv> Although of course that's long since been released and become devel, and stable, etc.
[15:56] <jtv> henninge: why do you introduce the imported1 and imported2 variables?  Are they used somewhere?  If not, they'll just cause lint warnings.
[15:57] <henninge> jtv: they are used in the assignment right after it.
[15:57] <jtv> OK
[15:57] <henninge> the tests were breaking because they were missing.
[16:06] <jtv> henninge: I've approved your MP.
[16:06] <henninge> jtv: thanks ;)
[16:06] <jtv> np
[16:23] <abentley> rockstar, I can has review? https://code.launchpad.net/~abentley/launchpad/config-disable-build-requests/+merge/27172
[16:39] <gary_poster> 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] <sinzui> gary_poster, you should use push() and pop() when working with the config
[16:41] <gary_poster> sinzui: but I don't think I can do that with changes of that variable, can I?
[16:42] <sinzui> push()  is intended to change one or more variables
[16:42] <sinzui> that is how we test instances
[16:42] <gary_poster> OK, will try.
[16:43] <abentley> EdwinGrubbs, could you please review https://code.launchpad.net/~abentley/launchpad/config-disable-build-requests/+merge/27172 ?
[16:43] <sinzui> gary_poster, look at lib/canonical/launchpad/pagetests/basic/demo-and-lpnet.txt, line 88
[16:44] <EdwinGrubbs> abentley, is it urgent, or can you wait until I finish another review.
[16:44] <sinzui> gary_poster, the teardown is on line 141
[16:44] <gary_poster> ack, looking, thank you
[16:47] <abentley> EdwinGrubbs, not urgent.
[17:22] <gary_poster> 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] <EdwinGrubbs> noodles775, where is the build.duration attribute defined? I don't see it in BinaryPackageBuild or its superclasses.
[17:25] <noodles775> EdwinGrubbs: lp.buildmaster.model.buildfarmjob (it returns None if either of the values are None).
[17:25] <noodles775> (it's via delegation rather than inheritance)
[17:30] <EdwinGrubbs> noodles775, r=me
[17:30] <noodles775> Thanks EdwinGrubbs
[18:11] <sinzui> gary_poster, r=me
[18:11] <gary_poster> thank you sinzui!
[19:38] <EdwinGrubbs> 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] <abentley> EdwinGrubbs, why?
[19:40] <EdwinGrubbs> 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] <EdwinGrubbs> abentley, even when build_from_branch.enabled=False, the request_builds link is active.
[19:41] <abentley> EdwinGrubbs, we have disabled any way to navigate to that form.  The only way you can get there is by URL hacking.
[19:41] <abentley> EdwinGrubbs, going to the +index page is URL hacking.
[19:41] <EdwinGrubbs> abentley, ok, r=me
[19:42] <EdwinGrubbs> abentley, oh, I didn't see that it has already been reviewed and approved.
[20:05] <deryck> Hi EdwinGrubbs.  I have a branch for review I'd like to get on the queue.  but I'm EODing soon.
[20:05] <deryck> EdwinGrubbs, can we start now and finish offline, or would you rather I ping someone else later?
[20:27] <deryck> EdwinGrubbs, never mind (for when you're available again).  I'll worry about review later.  Thanks, anyway.
[21:56] <abentley> rockstar, https://code.launchpad.net/~abentley/launchpad/recipe-build-email/+merge/27199
[23:44] <wgrant> 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.