/srv/irclogs.ubuntu.com/2009/12/01/#launchpad-reviews.txt

thumperhttps://code.edge.launchpad.net/~thumper/launchpad/fix-date-review-requested/+merge/15464 anyone?01:58
* mwhudson looks01:59
thumpermwhudson: thanks01:59
mwhudsonthumper: will sort_order now always be non-None?02:38
thumpermwhudson: yes02:38
mwhudsonit seems like it would be if you could create a review in the WIP state02:38
thumpermwhudson: but then it wouldn't be in the list02:39
thumpermwhudson: so it won't need to be sorted02:39
* thumper is heading to get shoes for the girls02:39
thumperback later02:39
mwhudsonthumper: ah ok02:39
thumpermwhudson: I could add another fallback to date_created if you think it is worthwhile02:41
mwhudsonthumper: nah02:41
mwhudsonthumper: reviewed 02:42
thumperta02:42
mwhudsonthumper: if you get the chance: https://code.edge.launchpad.net/~mwhudson/launchpad/code-import-paranoia/+merge/1546703:20
noodles775Hey gmb, I assume you're doing bugs RC stuff today and would prefer not to be reviewing non-urgent non RC branches?10:01
gmbnoodles775: Pretty much, yes. I might have a chance to take a look at anything like that later, though.10:07
noodles775gmb, thanks. There's no rush for this one at all, so just see how you go.10:11
=== gmb changed the topic of #launchpad-reviews to: on-call: gmb || reviewing: - || queue [] || Currently reviewing urgent and RC branches only || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara-afk is now known as matsubara
bacmorning gmb11:33
gmbHi bac11:34
baci'll be reviewing today but not until 1700UTC11:34
=== noodles775 changed the topic of #launchpad-reviews to: on-call: gmb || reviewing: - || queue [noodles] || Currently reviewing urgent and RC branches only || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
* noodles775 pops one in the queue just for whoever has time later (not urgent at all)11:37
noodles775https://code.edge.launchpad.net/~michael.nelson/launchpad/487009-db-generalise-ibuilder-1b/+merge/1547711:40
BjornT_gmb: can you review this small patch? it's an rc candidate: https://code.edge.launchpad.net/~bjornt/launchpad/choiceedit-click-handler/+merge/1548412:24
gmbBjornT_: Sure. I'll take a look at it in a couple of minutes.12:24
BjornT_thanks12:24
gmbBjornT_: r=me12:27
BjornT_gmb: thanks!12:28
salgadohttps://code.edge.launchpad.net/~stub/launchpad/bug-490239-session-tuning/+merge/1540312:29
salgadohttps://code.edge.launchpad.net/~stub/launchpad/auth-mirror-tables/+merge/1541412:29
salgadogmb, it'd be great if you could take the two above ones as well; both are RC candidates12:29
gmbsalgado: Will take a look at those after lunch.12:30
=== gmb changed the topic of #launchpad-reviews to: on-call: gmb || reviewing: - || queue [stub(rc), stub(rc), noodles] || Currently reviewing urgent and RC branches only || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
gmbsalgado: Also, https://code.edge.launchpad.net/~gmb/launchpad/indexed-message-parents/+merge/15486 is an RC candidate. adeuring is reviewing it now.12:31
gmbsalgado: It's a fix for bug 39409712:31
mupBug #394097: accessing message 0 of bug 371281 gives wrong return type of IMessage <api> <bug-page> <oops> <Launchpad Bugs:Triaged by allenap> <https://launchpad.net/bugs/394097>12:31
salgadogmb, cool, thanks for fixing that one!12:31
gmbsalgado: Working it out was mostly allenap's work, I assure you. I just contributed the patch to stop it from happening.12:32
gmbsalgado: Do you want me to request the rc review now or wait until adeuring is done with it?12:33
salgadogmb, it's ok to request it now12:33
gmbOk12:33
gmbARGH, bloody person picker.12:34
gmbsalgado: Requested. Doesn't say "RC" but I think you can work that out.12:34
salgadoyep, I can12:34
gmbadeuring: Note: the docstring for TestBugIndexedMessages is bong; fixing it now.12:35
salgadogmb, do you still need an RC review for https://code.edge.launchpad.net/~gmb/launchpad/bugzilla3.4-see-also-bug-419134/+merge/15404 ?13:39
gmbsalgado: No; QAing was too much of a pain, so we'll find a better way to do QA and CP it later.13:39
gmb(If absolutely necessary)13:39
salgadook13:40
=== mrevell is now known as mrevell-lunch
=== mrevell-lunch is now known as mrevell
gmbsalgado: I've made the cahnges you requested.14:15
=== gmb changed the topic of #launchpad-reviews to: on-call: gmb || reviewing: stub(rc) || queue [stub(rc), noodles] || Currently reviewing urgent and RC branches only || 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: gmb || reviewing: stub(rc) || queue [noodles] || Currently reviewing urgent and RC branches only || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
gmbHurrah for branches that are all removals.14:16
gmbBoo for branches that contain sampledata changes14:19
gmbsalgado: Should I land my branch on devel or db-devel?14:19
salgadogmb, devel14:19
gmbCool14:20
gmbsalgado: I'm a little leery of approving https://code.edge.launchpad.net/~stub/launchpad/auth-mirror-tables/+merge/15414. Not because I don't think it looks right - so far, so good - but I don't have the SQL nouse to spot errors, I suspect.14:26
salgadogmb, I know how that feels, but I find it unlikely we have much people that can spot errors on SQL written by our DBA14:29
gmbsalgado: Heh. Fair point. Just wanted to make you aware of it :)14:29
=== matsubara is now known as matsubara-lunch
=== gmb changed the topic of #launchpad-reviews to: on-call: gmb || reviewing: noodles || queue [] || Currently reviewing urgent and RC branches only || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== noodles775_ is now known as noodles775
gmbnoodles775: r=me with a few minor changes.15:30
=== gmb changed the topic of #launchpad-reviews to: on-call: gmb || reviewing: - || queue [] || Currently reviewing urgent and RC branches only || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775Thanks gmb!15:30
gmbnp15:30
=== salgado is now known as salgado-lunch
=== gmb 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
=== salgado-lunch is now known as salgado
=== bac changed the topic of #launchpad-reviews to: on-call: bac || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== deryck is now known as deryck[lunch]
bacsalgado: do you want to have a look at https://code.edge.launchpad.net/~thumper/launchpad/fix-date-review-requested/+merge/1546418:38
salgadobac, sure, I'll review it later18:41
=== danilo_ is now known as danilos
=== deryck[lunch] is now known as deryck
bacsalgado: another: https://code.edge.launchpad.net/~stub/launchpad/auth-mirror-tables/+merge/1541420:03
salgadobac, reviewing that one right now. :)20:03
bacsalgado: great.  just doing clean up.20:03
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk
=== sinzui changed the topic of #launchpad-reviews to: on-call: bac || reviewing: - || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
ursulabac, hi :)23:12
bachi ursula23:13
=== bac changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
ursulaoh :(23:14
bacursula: what's up?23:14
bacursula: you have a review?  np23:15
bacursula: at line 53 of the diff, please factor out the test for 'if product_series'23:19
ursulabac, let me see23:22
bacursula: this isn23:22
bacursula: this isn't for an RC is it?23:22
ursulabac, nope, I just wanted to have a review before going on vacations23:23
bacursula: ok23:23
bacthat code change is one thing23:23
ursulabac, what do you mean by factor out the test?23:24
bacursula: does this new field show up in the UI?23:24
bacursula: i mean:23:25
bacif product_series is not None:23:25
bac     if blah:23:25
bac       do something23:25
ursulaah, ok :)23:25
bac    elif23:25
bac    do something else23:25
bacursula: so how is this field set?  is it in the UI?23:26
ursulabac, yes, in the +changetranslators one23:26
ursulaI think I mentioned that in the MP..23:26
bacursula: right, i see that now.23:27
bacursula: you're going to need some tests for this.  a doc test, a story, and webservice too.23:28
ursulabac, I see23:28
ursulabac, well, I guess I'll have to do that when I return... or someday in this meantime23:29
bacursula: ok.  sorry.23:29
bacursula: you'll also need a schema change, no?23:32
bacursula: which will require a db review from jml and stub.23:32
ursulabac, already did that, in a separated branch, that has already landed23:33
bacursula: excellent!23:33
ursulabac, do you know an example so I can write the tests that are missing?23:34
ursulaI'll try to do that while I wait at the airport23:34
bacursula: i'd look at how some of the companion fields are tested, like translationpermission23:36
bacursula: that should give you an idea.23:37
ursulabac, awesome, thanks muchly23:37
bacursula: have a nice vacation!23:37
=== EdwinGrubbs is now known as Edwin-afk

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!