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