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