=== 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 [11:38] henninge, can I trouble you for a review? [11:38] https://code.edge.launchpad.net/~jtv/launchpad/bug-599254/+merge/28618 [11:39] jtv: I am onto it! === 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 [11:39] great, thanks [11:48] jtv: 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] henninge: you mean the one about the validators? [11:48] jtv: That's "These validators only ..." [11:48] yes, [11:48] That's what causes the problem. [11:49] Changes to the potemplate field are combined with changes to the flag, confusing the validators. [11:50] I don't understand. When are they combined? [11:51] jtv: I understand the paragraph (without the last sentence) to say: "The validator only looks at diverged messages." [11:52] Ah 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] ah, ok [11:53] so they either search all shared messages or all diverged ones. [12:00] Well, "all" diverged ones for the same template. [12:00] (and language and potmsgset, of course) [12:01] henninge: hi, I can haz review plz === 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__ [13:04] bigjools: sure === 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 [13:12] I has a couple of reviews up from the weekend [13:12] can I tempt anyone [13:13] sorry, I've neglected my prior obligations too much already today. [13:14] lifeless, fwiw, you won't be able to land them until Monday anyway. [13:14] jml: I'm not setup to land these days anyway [13:14] jml: my hope is to have someone review them overnight [13:14] lifeless, we won't be able to land them until Monday, then. [13:14] I can then make any changes, and after that they can queue as needed. [13:14] sure [13:14] thank you for calling that to my attention. [13:14] its a very foreign experience for me ;) === matsubara-afk is now known as matsubara [14:03] henninge: can I add another branch to the queue? [14:13] rinze: go ahead ;) === 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/ || [14:22] bigjools: does your security adapter really loop over all private PPAs in Launchpad? Couldn't become a performance issue? [14:22] that [14:22] henninge: just private PPAs for the person, not all [14:23] bigjools: I don't see the person mentioned, though: "getUtility(IArchiveSet).getPrivatePPAs()" [14:25] henninge: argh shit you're right [14:25] thanks for noticing [14:25] I'll see what I can do [14:28] bigjools: np, I'll push it back into the queue but for now but can approve it anytime you have a fix. === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: jelmer || queue: [bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || [14:29] rinze: on to yours, jelmer [14:29] henninge: ok thanks [15:40] henninge: pushed another revno up, but here's the partial diff: http://pastebin.ubuntu.com/456416/ [15:44] bigjools: Great usage of sets! [15:45] it reduces the extra query count to 2 :) [15:47] bigjools: r=me ;) [15:47] bigjools: you still need to remove the IArchiveSet import, I assume. [15:47] henninge: err yes :) [15:47] thanks :) === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: jelmer || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || [15:49] jtv: yeah, you can land the oops fix! ;) [16:00] henninge: yay! [16:00] henninge: on db-devel, right? [16:00] err .... [16:00] jtv: nope, devel. [16:01] henninge: isn't db-devel normal procedure after pqm closure? [16:01] no, after devel closure [16:01] argh [16:01] jtv: look at the #lp-code topic [16:02] henninge: right ho... that's easy then [16:02] (though I had a db-devel branch ready as well) === 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/ || [16:34] rinze: done! Have a look at it after the match. ;-) [16:36] rinze: Thanks! [16:36] rinze: talking to yourself ;) [16:36] * henninge leaves [16:37] henninge: Oops! Thank *you* [17:17] abentley: Hi, can I add a branch to the queue? [17:17] rinze, sure. === rinze changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || [17:18] abentley: thanks === 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/ || [19:40] abentley, can I have you look at this diff to fix the build. [19:41] http://pastebin.ubuntu.com/456525/ [19:41] rockstar, r=me. [19:41] abentley, thanks. [19:42] rockstar, let me guess: someone removed it and someone added something that needed it. [19:42] abentley, nope, just got removed. [19:42] rockstar, that's crazy. [19:43] abentley, I think it appeared to lint like it was an unused import, but it breaks the build entirely... [19:43] rockstar, yeah, but people are supposed to run the full test suite before landing things... [19:43] abentley, especially when it's landed as release-critical. [19:52] abentley: thanks for the review! [19:53] abentley, ping, could you please review this one line testfix? https://code.edge.launchpad.net/~mars/launchpad/testfix-missing-iarchive-import/+merge/28665 [19:54] mars, that looks just like the one rockstar just got me to review. [19:55] abentley, then good thing I didn't r=jfdi :) [19:56] I'll toss mine then [19:58] mars, mine's already in PQM, so I win anyway. [19:58] mars, as release manager though, could you please track down why a change like that was landed with the release-critical tag? [19:59] rockstar, three experienced reviewers trusted the original submitter - possibly a failure of impartial judgement on our parts [20:00] mars, it breaks the build, which means it didn't get a successful test run. [20:00] mars, 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 [20:02] abentley, this fix I made allowed Launchpad to build itself. It wouldn't even build to run tests without my change. [20:02] if you tried running 'make', that is [20:03] mars, or tried to do anything with zope... [20:03] if you just used bin/test or bin/buildout, as I did, then everything builds fine [20:04] mars, bin/test was failing for me. [20:05] mars, I suspect it matters what layer you're trying to run your test in though. [20:06] Hmm, bin/test -cvv test_archive_subscriptions failed when that line was missing [20:07] Again, 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 cover [20:08] rockstar, I can hit the list with a root cause analysis mail [20:09] mars, 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:10] mars, 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] rockstar, 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:11] * rockstar nods [20:12] rockstar, 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. === 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