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

=== jelmer_ is now known as Guest71703
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
jtvhenninge, can I trouble you for a review?11:38
jtvhttps://code.edge.launchpad.net/~jtv/launchpad/bug-599254/+merge/2861811:38
henningejtv: I am onto it!11:39
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
jtvgreat, thanks11:39
henningejtv: I don't understand why paragraph 3 is there. You don't seem to be doing anything about this situation. Or am I misreading it?11:48
jtvhenninge: you mean the one about the validators?11:48
henningejtv: That's "These validators only ..."11:48
henningeyes,11:48
jtvThat's what causes the problem.11:48
jtvChanges to the potemplate field are combined with changes to the flag, confusing the validators.11:49
henningeI don't understand. When are they combined?11:50
henningejtv: I understand the paragraph (without the last sentence) to say: "The validator only looks at diverged messages."11:51
jtvAh right, I missed something I had wanted to keep distinct: the validators only look at messages _with the same value in the potemplate field_, not for the same template as I said.11:52
henningeah, ok11:52
henningeso they either search all shared messages or all diverged ones.11:53
jtvWell, "all" diverged ones for the same template.12:00
jtv(and language and potmsgset, of course)12:00
bigjoolshenninge: hi, I can haz review plz12:01
=== bigjools changed the topic of #launchpad-reviews to: On call: henninge || reviewing: jtv || queue: [bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== Guest71703 is now known as jelmer_
=== jelmer_ is now known as Guest58868
=== Guest58868 is now known as jelmer__
henningebigjools: sure13:04
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: bigjools || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
lifelessI has a couple of reviews up from the weekend13:12
lifelesscan I tempt anyone13:12
jmlsorry, I've neglected my prior obligations too much already today.13:13
jmllifeless, fwiw, you won't be able to land them until Monday anyway.13:14
lifelessjml: I'm not setup to land these days anyway13:14
lifelessjml: my hope is to have someone review them overnight13:14
jmllifeless, we won't be able to land them until Monday, then.13:14
lifelessI can then make any changes, and after that they can queue as needed.13:14
lifelesssure13:14
lifelessthank you for calling that to my attention.13:14
lifelessits a very foreign experience for me ;)13:14
=== matsubara-afk is now known as matsubara
rinzehenninge: can I add another branch to the queue?14:03
henningerinze: go ahead ;)14:13
=== rinze changed the topic of #launchpad-reviews to: On call: henninge || reviewing: bigjools || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || h
=== rinze changed the topic of #launchpad-reviews to: On call: henninge || reviewing: bigjools || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ ||
henningebigjools: does your security adapter really loop over all private PPAs in Launchpad? Couldn't become a performance issue?14:22
henningethat14:22
bigjoolshenninge: just private PPAs for the person, not all14:22
henningebigjools: I don't see the person mentioned, though: "getUtility(IArchiveSet).getPrivatePPAs()"14:23
bigjoolshenninge: argh shit you're right14:25
bigjoolsthanks for noticing14:25
bigjoolsI'll see what I can do14:25
henningebigjools: np, I'll push it back into the queue but for now but can approve it anytime you have a fix.14:28
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: jelmer || queue: [bigjools] || This channel is logged: http://irclogs.ubuntu.com/ ||
henningerinze: on to yours, jelmer14:29
bigjoolshenninge: ok thanks14:29
bigjoolshenninge: pushed another revno up, but here's the partial diff: http://pastebin.ubuntu.com/456416/15:40
henningebigjools: Great usage of sets!15:44
bigjoolsit reduces the extra query count to 2 :)15:45
henningebigjools: r=me ;)15:47
henningebigjools: you still need to remove the IArchiveSet import, I assume.15:47
bigjoolshenninge: err yes :)15:47
bigjoolsthanks :)15:47
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: jelmer || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ ||
henningejtv: yeah, you can land the oops fix! ;)15:49
jtvhenninge: yay!16:00
jtvhenninge: on db-devel, right?16:00
henningeerr ....16:00
henningejtv: nope, devel.16:00
jtvhenninge: isn't db-devel normal procedure after pqm closure?16:01
henningeno, after devel closure16:01
jtvargh16:01
henningejtv: look at the #lp-code topic16:01
jtvhenninge: right ho... that's easy then16:02
jtv(though I had a db-devel branch ready as well)16:02
=== abentley changed the topic of #launchpad-reviews to: On call: henninge, abentley || reviewing: jelmer, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ ||
=== henninge changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ ||
henningerinze: done! Have a look at it after the match. ;-)16:34
rinzerinze: Thanks!16:36
henningerinze: talking to yourself ;)16:36
* henninge leaves16:36
rinzehenninge: Oops! Thank *you*16:37
rinzeabentley: Hi, can I add a branch to the queue?17:17
abentleyrinze, sure.17:17
=== rinze changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ ||
rinzeabentley: thanks17:18
=== mrevell is now known as mrevell-dinner
=== matsubara is now known as matsubara-afk
=== matsubara-afk is now known as matsubara-lunch
=== salgado is now known as salgado-lunch
=== abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ ||
rockstarabentley, can I have you look at this diff to fix the build.19:40
rockstarhttp://pastebin.ubuntu.com/456525/19:41
abentleyrockstar, r=me.19:41
rockstarabentley, thanks.19:41
abentleyrockstar, let me guess: someone removed it and someone added something that needed it.19:42
rockstarabentley, nope, just got removed.19:42
abentleyrockstar, that's crazy.19:42
rockstarabentley, I think it appeared to lint like it was an unused import, but it breaks the build entirely...19:43
abentleyrockstar, yeah, but people are supposed to run the full test suite before landing things...19:43
rockstarabentley, especially when it's landed as release-critical.19:43
rinzeabentley: thanks for the review!19:52
marsabentley, ping, could you please review this one line testfix? https://code.edge.launchpad.net/~mars/launchpad/testfix-missing-iarchive-import/+merge/2866519:53
abentleymars, that looks just like the one rockstar just got me to review.19:54
marsabentley, then good thing I didn't r=jfdi :)19:55
marsI'll toss mine then19:56
rockstarmars, mine's already in PQM, so I win anyway.19:58
rockstarmars, as release manager though, could you please track down why a change like that was landed with the release-critical tag?19:58
marsrockstar, three experienced reviewers trusted the original submitter - possibly a failure of impartial judgement on our parts19:59
rockstarmars, it breaks the build, which means it didn't get a successful test run.20:00
abentleymars, rockstar, I have a fairly good idea: julian was already getting test failures because of bug #599397, so he didn't notice the legitimate failures.20:00
_mup_Bug #599397: code imports break when tdb is installed <code-import> <Launchpad Bazaar Integration:Triaged> <https://launchpad.net/bugs/599397>20:00
rockstarabentley, this fix I made allowed Launchpad to build itself.  It wouldn't even build to run tests without my change.20:02
marsif you tried running 'make', that is20:02
rockstarmars, or tried to do anything with zope...20:03
marsif you just used bin/test or bin/buildout, as I did, then everything builds fine20:03
rockstarmars, bin/test was failing for me.20:04
rockstarmars, I suspect it matters what layer you're trying to run your test in though.20:05
marsHmm, bin/test -cvv test_archive_subscriptions failed when that line was missing20:06
marsAgain, as one of the reviewers, I know there I was in error to assume that ec2 test would be run, and that the missing line was intentional but unmentioned in the cover20:07
marsrockstar, I can hit the list with a root cause analysis mail20:08
rockstarmars, I don't think as a reviewer you should have to ask for the test suite to have been run, because it was landed with [release-critical]20:09
rockstarmars, we make mistakes and land broken code sometimes.  Human error should be allowed.  When things are landed rc, I expect that we've tried to account for as any human errors as possible.20:10
marsrockstar, but I am also the release-critical gatekeeper - I should have ensured the run (as I have explicitly asked for in the last two r-c's I gave out.)20:10
* rockstar nods20:11
marsrockstar, I still think a root cause analysis mail is worthwhile here.  We only know one (me) and a half (bigjools via abentley) sides of the story.20:12
=== matsubara-lunch is now known as matsubara
=== salgado-lunch is now known as salgado
=== _thumper_ is now known as thumper
=== salgado is now known as salgado-afk
=== matsubara is now known as matsubara-afk

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