=== 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 | ||
jtv | henninge, can I trouble you for a review? | 11:38 |
---|---|---|
jtv | https://code.edge.launchpad.net/~jtv/launchpad/bug-599254/+merge/28618 | 11:38 |
henninge | jtv: 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 | ||
jtv | great, thanks | 11:39 |
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:48 |
jtv | Changes to the potemplate field are combined with changes to the flag, confusing the validators. | 11:49 |
henninge | I don't understand. When are they combined? | 11:50 |
henninge | jtv: I understand the paragraph (without the last sentence) to say: "The validator only looks at diverged messages." | 11:51 |
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:52 |
henninge | so they either search all shared messages or all diverged ones. | 11:53 |
jtv | Well, "all" diverged ones for the same template. | 12:00 |
jtv | (and language and potmsgset, of course) | 12:00 |
bigjools | henninge: hi, I can haz review plz | 12: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__ | ||
henninge | bigjools: sure | 13: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 | ||
lifeless | I has a couple of reviews up from the weekend | 13:12 |
lifeless | can I tempt anyone | 13:12 |
jml | sorry, I've neglected my prior obligations too much already today. | 13:13 |
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 ;) | 13:14 |
=== matsubara-afk is now known as matsubara | ||
rinze | henninge: can I add another branch to the queue? | 14:03 |
henninge | rinze: 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/ || | ||
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:22 |
henninge | bigjools: I don't see the person mentioned, though: "getUtility(IArchiveSet).getPrivatePPAs()" | 14:23 |
bigjools | henninge: argh shit you're right | 14:25 |
bigjools | thanks for noticing | 14:25 |
bigjools | I'll see what I can do | 14:25 |
henninge | bigjools: 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/ || | ||
henninge | rinze: on to yours, jelmer | 14:29 |
bigjools | henninge: ok thanks | 14:29 |
bigjools | henninge: pushed another revno up, but here's the partial diff: http://pastebin.ubuntu.com/456416/ | 15:40 |
henninge | bigjools: Great usage of sets! | 15:44 |
bigjools | it reduces the extra query count to 2 :) | 15:45 |
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:47 |
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: jelmer || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || | ||
henninge | jtv: yeah, you can land the oops fix! ;) | 15:49 |
jtv | henninge: yay! | 16:00 |
jtv | henninge: on db-devel, right? | 16:00 |
henninge | err .... | 16:00 |
henninge | jtv: nope, devel. | 16:00 |
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:01 |
jtv | henninge: right ho... that's easy then | 16: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/ || | ||
henninge | rinze: done! Have a look at it after the match. ;-) | 16:34 |
rinze | rinze: Thanks! | 16:36 |
henninge | rinze: talking to yourself ;) | 16:36 |
* henninge leaves | 16:36 | |
rinze | henninge: Oops! Thank *you* | 16:37 |
rinze | abentley: Hi, can I add a branch to the queue? | 17:17 |
abentley | rinze, 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/ || | ||
rinze | abentley: thanks | 17: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/ || | ||
rockstar | abentley, can I have you look at this diff to fix the build. | 19:40 |
rockstar | http://pastebin.ubuntu.com/456525/ | 19:41 |
abentley | rockstar, r=me. | 19:41 |
rockstar | abentley, thanks. | 19:41 |
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:42 |
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:43 |
rinze | abentley: thanks for the review! | 19:52 |
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:53 |
abentley | mars, that looks just like the one rockstar just got me to review. | 19:54 |
mars | abentley, then good thing I didn't r=jfdi :) | 19:55 |
mars | I'll toss mine then | 19:56 |
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:58 |
mars | rockstar, three experienced reviewers trusted the original submitter - possibly a failure of impartial judgement on our parts | 19:59 |
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:00 |
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:02 |
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:03 |
rockstar | mars, bin/test was failing for me. | 20:04 |
rockstar | mars, I suspect it matters what layer you're trying to run your test in though. | 20:05 |
mars | Hmm, bin/test -cvv test_archive_subscriptions failed when that line was missing | 20:06 |
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:07 |
mars | rockstar, I can hit the list with a root cause analysis mail | 20:08 |
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:09 |
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:10 |
* rockstar nods | 20:11 | |
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. | 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!