=== Ursinha is now known as Ursinha-afk | ||
=== matsubara is now known as matsubara-afk | ||
=== salgado is now known as salgado-lunch | ||
deryck | hmmm, it seems no one is on call reviewer today. | 12:52 |
---|---|---|
deryck | adeuring, could I beg a review of you? | 12:53 |
deryck | warning that it's a long diff but a simple change -- moving duplicateof to markAsDuplicate. | 12:54 |
adeuring | deryck: sure | 13:00 |
adeuring | sorry, was out for lunch | 13:00 |
deryck | adeuring, no worries. And thanks! See: https://code.edge.launchpad.net/~deryck/launchpad/do-the-right-thing-dupe-move-78596/+merge/27144 | 13:01 |
adeuring | deryck: in BugMarkAsDuplicateView.setUpFields(), you set self.form_fields twice. Can't we remove the first assignment? | 13:18 |
=== 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 | ||
deryck | adeuring, let me look at the diff... | 13:18 |
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:19 |
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:20 |
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:21 |
adeuring | sounds good | 13:22 |
adeuring | deryck: r=me, with 1.5 comments | 14:07 |
deryck | adeuring, cool. thanks! | 14:08 |
=== 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 | ||
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:51 |
EdwinGrubbs | I'll start on it soon. | 14:53 |
=== leonardr is now known as leonardr-afk | ||
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:51 |
jtv | henninge: hope the drive-bys won't cause any conflicts. | 15:53 |
jtv | Or not too many, at any rate. | 15:53 |
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:54 |
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:55 |
jtv | henninge: why do you introduce the imported1 and imported2 variables? Are they used somewhere? If not, they'll just cause lint warnings. | 15:56 |
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. | 15:57 |
jtv | henninge: I've approved your MP. | 16:06 |
henninge | jtv: thanks ;) | 16:06 |
jtv | np | 16:06 |
=== 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 | ||
abentley | rockstar, I can has review? https://code.launchpad.net/~abentley/launchpad/config-disable-build-requests/+merge/27172 | 16:23 |
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:39 |
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:41 |
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:42 |
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:43 |
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:44 |
abentley | EdwinGrubbs, not urgent. | 16:47 |
=== matsubara is now known as matsubara-lunch | ||
=== mwhudson_ is now known as mwhudson | ||
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:22 |
EdwinGrubbs | noodles775, where is the build.duration attribute defined? I don't see it in BinaryPackageBuild or its superclasses. | 17:24 |
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:25 |
EdwinGrubbs | noodles775, r=me | 17:30 |
noodles775 | Thanks EdwinGrubbs | 17:30 |
=== 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 | ||
sinzui | gary_poster, r=me | 18:11 |
gary_poster | thank you sinzui! | 18:11 |
=== deryck[lunch] is now known as deryck | ||
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:38 |
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:40 |
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:41 |
EdwinGrubbs | abentley, oh, I didn't see that it has already been reviewed and approved. | 19:42 |
=== 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 | ||
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:05 |
deryck | EdwinGrubbs, never mind (for when you're available again). I'll worry about review later. Thanks, anyway. | 20:27 |
abentley | rockstar, https://code.launchpad.net/~abentley/launchpad/recipe-build-email/+merge/27199 | 21:56 |
=== matsubara is now known as matsubara-afk | ||
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. | 23:44 |
=== 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 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!