/srv/irclogs.ubuntu.com/2010/06/09/#launchpad-reviews.txt

=== Ursinha is now known as Ursinha-afk
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-lunch
deryckhmmm, it seems no one is on call reviewer today.12:52
deryckadeuring, could I beg a review of you?12:53
deryckwarning that it's a long diff but a simple change -- moving duplicateof to markAsDuplicate.12:54
adeuringderyck: sure13:00
adeuringsorry, was out for lunch13:00
deryckadeuring, no worries.  And thanks!  See:  https://code.edge.launchpad.net/~deryck/launchpad/do-the-right-thing-dupe-move-78596/+merge/2714413:01
adeuringderyck: 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
deryckadeuring, let me look at the diff...13:18
noodles775Well-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/2714713:19
adeuringderyck: zthatÄs lines 54, 55 of the diff13:19
deryckadeuring, 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
deryckcribbed, rather13:21
adeuringderyck: admitteldy, i am not sure... don't know enough about the "field magic"...13:21
deryckadeuring, I can try it and test.  If so, I'll change it.13:21
adeuringsounds good13:22
adeuringderyck: r=me, with 1.5 comments14:07
deryckadeuring, 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
noodles775EdwinGrubbs: 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/2714714:51
EdwinGrubbsI'll start on it soon.14:53
=== leonardr is now known as leonardr-afk
henningejtv: can you please look at this before you go?15:51
henningehttps://code.edge.launchpad.net/~henninge/launchpad/bug-591731-is-published-removal/+merge/2716615:51
jtvhenninge: looking...15:51
henningejtv: it's mostly mechanical15:51
jtvhenninge: get a chance to look at the wiki update?15:51
henningejtv: no, will do that now.15:51
jtvThanks.15:51
jtvhenninge: hope the drive-bys won't cause any conflicts.15:53
jtvOr not too many, at any rate.15:53
henningejtv: I plan to merge stable (or was it db_stable) after my message sharing branch has landed.15:54
jtvhenninge: 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
jtvrecife was based on db-stable.15:55
henningejtv: ah, good idea15:55
jtvAlthough of course that's long since been released and become devel, and stable, etc.15:55
jtvhenninge: why do you introduce the imported1 and imported2 variables?  Are they used somewhere?  If not, they'll just cause lint warnings.15:56
henningejtv: they are used in the assignment right after it.15:57
jtvOK15:57
henningethe tests were breaking because they were missing.15:57
jtvhenninge: I've approved your MP.16:06
henningejtv: thanks ;)16:06
jtvnp16: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
abentleyrockstar, I can has review? https://code.launchpad.net/~abentley/launchpad/config-disable-build-requests/+merge/2717216:23
gary_postersinzui: 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/2717416:39
sinzuigary_poster, you should use push() and pop() when working with the config16:41
gary_postersinzui: but I don't think I can do that with changes of that variable, can I?16:41
sinzuipush()  is intended to change one or more variables16:42
sinzuithat is how we test instances16:42
gary_posterOK, will try.16:42
abentleyEdwinGrubbs, could you please review https://code.launchpad.net/~abentley/launchpad/config-disable-build-requests/+merge/27172 ?16:43
sinzuigary_poster, look at lib/canonical/launchpad/pagetests/basic/demo-and-lpnet.txt, line 8816:43
EdwinGrubbsabentley, is it urgent, or can you wait until I finish another review.16:44
sinzuigary_poster, the teardown is on line 14116:44
gary_posterack, looking, thank you16:44
abentleyEdwinGrubbs, not urgent.16:47
=== matsubara is now known as matsubara-lunch
=== mwhudson_ is now known as mwhudson
gary_postersinzui: 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
EdwinGrubbsnoodles775, where is the build.duration attribute defined? I don't see it in BinaryPackageBuild or its superclasses.17:24
noodles775EdwinGrubbs: 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
EdwinGrubbsnoodles775, r=me17:30
noodles775Thanks EdwinGrubbs17: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
sinzuigary_poster, r=me18:11
gary_posterthank you sinzui!18:11
=== deryck[lunch] is now known as deryck
EdwinGrubbsabentley, 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_builds19:38
abentleyEdwinGrubbs, why?19:38
EdwinGrubbsabentley, 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
EdwinGrubbsabentley, even when build_from_branch.enabled=False, the request_builds link is active.19:40
abentleyEdwinGrubbs, we have disabled any way to navigate to that form.  The only way you can get there is by URL hacking.19:41
abentleyEdwinGrubbs, going to the +index page is URL hacking.19:41
EdwinGrubbsabentley, ok, r=me19:41
EdwinGrubbsabentley, 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
deryckHi EdwinGrubbs.  I have a branch for review I'd like to get on the queue.  but I'm EODing soon.20:05
deryckEdwinGrubbs, can we start now and finish offline, or would you rather I ping someone else later?20:05
deryckEdwinGrubbs, never mind (for when you're available again).  I'll worry about review later.  Thanks, anyway.20:27
abentleyrockstar, https://code.launchpad.net/~abentley/launchpad/recipe-build-email/+merge/2719921:56
=== matsubara is now known as matsubara-afk
wgrantabentley: 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!