[00:07] rockstar: approved with tweaks [00:08] thumper: morning [00:08] hi mwhudson [00:08] thumper: did you see my discussion with gary earlier? [00:08] not really [00:08] I saw my nick flash by [00:08] but didn't read [00:08] whazzup? [00:08] thumper: i have a series of branches up for review, could you look at them? [00:08] how long are they? [00:09] thumper: https://code.edge.launchpad.net/~mwhudson/launchpad/+activereviews [00:09] heh [00:09] I just typed that in [00:09] 200-300 lines each [00:09] I see you have two reviews to do :) [00:09] vostok? [00:09] soyuz precursor? [00:10] thumper: yeah [00:12] mwhudson: the vostok apache config has /var/tmp/vostok-archive [00:12] mwhudson: but I don't see that path in the makefile [00:12] should it be there? [00:12] thumper: i guess so [00:12] * mwhudson didn't write that config [00:39] thumper: what is __used_for__ for? [00:39] I don't know... [00:39] just seen it lots [00:40] perhaps it is no longer used [00:40] in which case, add a docstring instead of pass :) [00:40] it's set a bunch of times in the codebase but never accesses [00:40] accessed rather [00:44] don't bother setting then... [00:44] I'm not sure if zope itself cares [00:44] if it did, we probably override that behaviour anyway [00:45] i grepped and googled and i think it's no longer used [00:45] ok [00:45] https://mail.zope.org/pipermail/zope3-users/2005-May/000563.html [00:45] just a docstring then... [00:45] """The view for the Vostok root object.""" [00:45] ? [00:45] yeah, I guess [00:46] one step better than pass [00:48] thumper: also i can't find get_initialized_view [00:48] i know there's something like that, but what's it called? [00:48] from lp.testing.views import create_initialized_view [00:49] sorry [00:49] thanks [00:49] mwhudson: BTW, did a count earlier, almost a third of the total tests start with lp.code :) [00:50] that skips all doctests as they don't have a module or path prefix [00:50] thumper: heh heh [01:18] mwhudson: I'm afk for a while, I'll check on your responses when I get back [01:18] thumper: thanks! [04:17] thumper: which Makefile target should create /var/tmp/vostok-archive ? [04:17] it's not like there's any pattern to follow about this sort of thing :( [04:18] hmm... [04:18] do we not create other dirs? [04:18] thumper: clean_codehosting creates a few [04:18] this doesn't seem to fit there :-) [04:19] actually I think most of clean_codehosting should go into build: [04:19] and the rest into clean [04:20] can i not do this now? :-) [04:21] i'll add a mkdir to build? [04:21] or compile i guess [04:21] yeah, just add it to one of those [04:24] thumper: did you have any more comments on https://code.edge.launchpad.net/~mwhudson/launchpad/vostok-add-root/+merge/31239 ? [04:24] thumper: i'm working on removing the need to do the launchbag thing [04:25] just one comment on the XXX comment [04:26] other than that, it is good [04:33] thumper: thanks [05:30] mwhudson: done your new review [05:41] thumper: thanks [05:48] thumper: i made some changes to -main-template and -traverse-distro can you look again? [05:48] ok [05:54] mwhudson: for -traverse-distro, can you add to the redirect test to test that the canonical url contains the vostok vhost? [05:54] so we know [05:54] thumper: ah yes, good idea [05:55] other than that it looks good [05:56] thumper: i'm surprised that test passes [05:56] i guess the testrequest must still be current in some sense [05:57] what do you mean? [06:00] thumper: the test calls canonical_url on the distro, and that must be returning the url on the vostok vhost [06:00] or [06:00] hm [06:00] bah [06:01] * thumper waits holding breath [06:01] the redirection is good in 'make run' [06:01] but not in the tst [06:01] test [06:04] oh [06:04] the test isn't using VostokTestRequest [06:05] nope, not that [06:15] /me boggles [06:16] even using test_traverse the redirect target is wrong [06:16] but in the webapp it's correct [06:17] aaaaaargh [06:18] hm [06:18] pizza and movie night is starting [06:18] I'll check in later [06:19] good call === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === stub1 is now known as stub === jelmer_ is now known as jelmer === mrevell is now known as mrevell-lunch === matsubara-afk is now known as matsubara === mrevell-lunch is now known as mrevell === bac changed the topic of #launchpad-reviews to: On call: adeuring,bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:00] hi adeuring. staying busy? [14:00] hi bac, up to now, I had no review request [14:01] i see a few on +activereviews. i'll grab some of those [14:31] hi adeurin, bac [14:31] sorry, hi adeuring [14:31] Any chance I can add a branch to the queue? [14:32] jelmer: sure -- hadn't yet reviewed today... [14:32] jelmer: where is your branch? [14:32] adeuring: https://code.edge.launchpad.net/~jelmer/launchpad/521110-use-python-debian-debversion/+merge/31268 [14:33] I think you reviewed a version of that branch a while ago (4 months?) [14:33] jelmer: yes, I remember -- I was a bit paranoid at that time ;) [14:34] https://code.edge.launchpad.net/~jelmer/launchpad/noversion/+merge/21059 was the MP from back then === adeuring changed the topic of #launchpad-reviews to: On call: adeuring,bac || reviewing: jelmer, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:34] There was one particular test that failed with older versions of python-apt. I've now landed an updated version of python-debian that works around that bug in the PPA. [14:34] ok [14:40] jelmer: your branch has conflicts: https://code.edge.launchpad.net/~jelmer/launchpad/buildd-trivial/+merge/30808 [14:41] bac: I'll fix - thanks [14:41] I guess the smart debian/changelog merger isn't running on lp yet :-) [14:41] * jelmer is waiting for somebody to come up with a smart 3-way python merger [14:53] jelmer: the new symlinks lib/deb822.py and lib/debian point to nowhere on my machine. [14:54] adeuring: Ah, sorry. I need to remove those as we ended up putting python-debian in the PPA rather than in sourcedeps.conf. === matsubara is now known as matsubara-lunch [14:55] jelmer: ok [15:07] james_w: ping [15:07] hi EdwinGrubbs [15:08] bac: The changelog conflict should be fixed now. [15:09] james_w: I'm not sure what was intended by the merge, but in my mind, the target archive could have a package that the source archive does not have, and then after the merge, the target archive is a union of both archives. The test just seems to update the target archive so that it is a mirror of the source archive. If that is all that it will be used for, there is no reason for more testing. I am just curious about the [15:09] purpose of merge. [15:09] jelmer: you rock and i take back all of those nasty things i thought about you yesterday === bac changed the topic of #launchpad-reviews to: On call: adeuring,bac || reviewing: jelmer,jml - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [15:10] jelmer: in Version.__init__(), is it OK if self.epoch or self.debian_version are None? [15:10] EdwinGrubbs: that is what a merge is, correct, but that behaviour is tested elsewhere. This was just intended to be a test that a merge happened. I'm happy to make the test more complex, but I'm not sure there is any value in showing that aspect of a merge at this level. [15:11] james_w: ok, if that's tested elsewhere, don't worry about it. land away. [15:11] EdwinGrubbs: thanks [15:11] bac: Yep, though it should be None rather than "" [15:12] sorry, s/bac/adeuring/ [15:12] bac: :-) [15:12] jelmer: ok, thanks [15:27] jelmer: r=me, but a few nitpicks === adeuring changed the topic of #launchpad-reviews to: On call: adeuring,bac || reviewing: -, jml || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [15:29] adeuring: Thanks! [16:16] jelmer: i am going to pass on reviewing https://code.edge.launchpad.net/~jelmer/launchpad/buildd-trivial/+merge/30808 [16:16] bac: ok [16:16] jelmer: it all looks reasonable, but i don't know enough about packagin [16:16] jelmer: perhaps bigjools can look [16:17] lamont should review it [16:17] he owns the code [16:18] bigjools: Ah, ok - I'll wait for him then. [16:18] bac, bigjools: Thanks [16:19] jelmer: hilariously, I'm sure wgrant recently removed some of the files you're re-adding [16:20] bigjools: Unfortunately there's a gazillion alternatives for doing some things in Debian packages :-/ [16:20] yay [16:57] jelmer: have you pinged lamont to review that branch? [16:58] bac: yeah, he's subscribed but on holiday at the moment [16:58] ok === salgado is now known as salgado-lunch [16:59] bac: There's no hurry though, these were just some tech-debt fixes. [16:59] jelmer: right === matsubara-lunch is now known as matsubara === adeuring changed the topic of #launchpad-reviews to: On call: bac || reviewing: jml || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === deryck is now known as deryck[lunch] [17:35] anybody around to review a one-liner? [17:36] bac: could I add it to your queue? [17:36] yep [17:37] it's at https://code.edge.launchpad.net/~jelmer/launchpad/jelmer-ec2-image-update/+merge/31388 === jelmer changed the topic of #launchpad-reviews to: On call: bac || reviewing: jml || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === salgado-lunch is now known as salgado === deryck[lunch] is now known as deryck [17:59] bac: Thanks for the review, have a good weekend! === Ursinha is now known as Ursinha-lunch === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === matsubara is now known as matsubara-brb === gary_poster_ is now known as gary_poster [19:47] hi bac: I am preparing a short branch to make the testrunner stop shouting at me for things I did not right. Will you have time to review it? [19:47] yes [19:48] 'right' is mispelled, yet I know I was 'right' in how I wrote the tests. === matsubara-brb is now known as matsubara === Ursinha-lunch is now known as Ursinha === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:24] rockstar, I can has review? https://code.edge.launchpad.net/~abentley/launchpad/detailed-build-emails/+merge/31413 [20:25] abentley, can you verify that there are no conflicts in your branch? [20:25] (Launchpad claims there is) [20:27] rockstar, there weren't, but now there are. [20:27] rockstar, pushing a fix. [20:28] rockstar, this is what I get for fixing lint. [20:30] sinzui: is your branch ready? [20:31] bac: in 10 minutes. I am waiting for a test to complete [20:31] ok [20:31] ping me when it is available? [20:31] rockstar, updated. [20:36] abentley, huh. [20:36] abentley, I can hear you, but you can't hear me... [20:36] abentley, one sec. [20:56] bac: I am stalled. I fixed a found a typo on the naked object. this broke the test. I am perplexed because I cannot see how the test passed before I fixed it [20:56] NGDGU [21:00] bac: I found the mistake. the assertion was wrong. I will take a moment to explain we were expecting a "demotion" in the cover letter [21:21] bac: My MP will be visible in a few minutes [21:21] ok [21:29] bac https://code.launchpad.net/~sinzui/launchpad/stop-shouting-0/+merge/31423 when you have time [21:30] looking now [22:03] jelmer: I actually just removed lots of non-packaging cruft, and was leaving the package cleanups for a subsequent branch. [22:03] wgrant: Ah, great.. so our work probably doesn't overlap then [22:03] No. [22:04] I just cleaned up some lintian warnings and added a 'prepare' target to debian/rules so I could use a custom builder === bac changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === matsubara is now known as matsubara-afk