[03:32] <lifeless> anyone got a sec? had ec2 find a new failure in my librarian branch; 1 line code change, 6 line test change (3 in two tests)
[03:33] <lifeless> http://bazaar.launchpad.net/~lifeless/launchpad/private-librarian/revision/9600
[03:33] <lifeless> thumper: ^
[04:17] <lifeless> thumper: hi ?
[04:18] <thumper> lifeless: hi
[04:18] <lifeless> can you eyeball http://bazaar.launchpad.net/~lifeless/launchpad/private-librarian/revision/9600 please
[04:18] <thumper> what does self._when_streaming() do?
[04:18] <thumper> also
[04:18] <thumper> why do you need to import the entire module ? import lp.services.features
[04:19] <lifeless> self._when_streaming is a hook point for subclasses, I added it earlier but forgot to call it :(
[04:19] <lifeless> it lets the subclass do $whatever, in this case add a header.
[04:20] <lifeless> I could do from lp.services import features if you like
[04:20] <lifeless> per_thread, the variable that needs to be poked, is a package scope variabe
[04:20] <lifeless> theres a bug, I'll add an XXX to it
[04:21] <thumper> you could make a null feature fixture
[04:21] <lifeless> we should just fix the bug
[04:21] <lifeless> https://bugs.edge.launchpad.net/launchpad-foundations/+bug/631884
[04:21] <_mup_> Bug #631884: feature flags get out of sync easily <Launchpad Foundations:New> <https://launchpad.net/bugs/631884>
[04:22] <thumper> ok
[04:22] <thumper> do you have an MP?
[04:22] <lifeless> yes
[04:22] <thumper> or just landing?
[04:22] <lifeless> its already review on the MP
[04:22] <thumper> ok
[04:22] <thumper> fine then
[04:22] <lifeless> this is fixing the MP because it found a bug
[04:23] <lifeless> if you know what I mean
[04:23] <thumper> I've hit a snag with my lp:project for private code
[04:23] <thumper> a snag in that it isn't working
[04:24] <mwhudson> thumper: :(
[04:24] <mwhudson> thumper: which bit isn't working?
[04:25] <lifeless> thumper: can we help?
[04:25] <thumper> mwhudson: only the bit that bzr calls to resolve lp:foo
[04:25] <thumper> possibly
[04:25] <thumper> if I go `bzr push bzr+ssh://bazaar.staging.launchpad.net/+branch/wikkid` it worked when I had privacy set and no trunk branch
[04:25] <thumper> but both push and pull fail with:
[04:26] <thumper> bzr: ERROR: Invalid url supplied to transport: "lp://staging/wikkid": Wikkid Wiki has no default branch.
[04:26] <thumper> which surprised me
[04:26] <thumper> I'm looking for the whole in the test coverage
[04:45] <thumper> I found the problem
[04:45] <thumper> the tests were there
[04:45] <thumper> but not checking for the right thing
[11:07] <thumper> any one on call?
[11:08] <thumper> https://code.edge.launchpad.net/~thumper/launchpad/xmlrpc-tests/+merge/34719
[11:08] <thumper> rollout blocker ^^^
[11:20] <thumper> done by gmb
[12:26] <noodles775> Hi henninge! If you'd like to, I've got a branch which will require a UI review at: A following branch will add the remaining non-js functionality (the search bar).
[12:26] <noodles775> grr
[12:26] <noodles775> https://code.edge.launchpad.net/~michael.nelson/launchpad/distro-series-difference-browser2/+merge/34739
[12:27] <noodles775> but no pressure, or rush... it can wait until someone else is available if you're busy.
[12:27] <henninge> noodles775: no, that's ok. I am already looking at it.
[12:28] <noodles775> Thanks henninge
[12:28]  * noodles775 takes a break for lunch.
[14:44] <jelmer> bac: Hi
[14:44] <bac> hi jelmer
[14:46] <jelmer> bac: I've (finally!) managed to fix up the tests for the branch I proposed on Friday, and I was wondering if you could have another look. Julian's also done a review this morning - we'd really like to get this in for the current release.
[14:47] <bac> jelmer: ok.
[14:48] <bac> jelmer: you have gotten all of the tests to work now?
[14:49] <jelmer> bac: Yes. You were indeed right that there were some failing; what I was running on dogfood was slightly more hackish (but working) than what was in the branch I had proposed.
[14:50] <bac> jelmer: i'd like to run the tests.  it'll take about 30 minutes.  that's ok for you?
[14:50] <jelmer> bac: yeah, np
[14:50] <bac> jelmer: ok.
[15:06] <bac> gmb: jelmer's branch is approved and awaits your RC approval:  https://code.edge.launchpad.net/~jelmer/launchpad/506256-remove-popen/+merge/34549
[15:07]  * bac was going to say "Imprimatur" but couldn't spell it.
[15:08] <bac> gmb: actually, wait a bit on that.  my testing may have been flawed
[15:09]  * gmb waits
[15:18] <bac> gmb: done.
[15:18] <gmb> bac, Looking now.
[15:21] <gmb> bac, jelmer : rc=me; please land it on db-devel if you can; we want to close devel for landings today.
[15:21] <gmb> (Once it's un-broken)
[15:21] <jelmer> gmb: ok
[15:21] <jelmer> bac, gmb: Thanks for the reviews.
[15:31] <noodles775> mars or leonardr: Could either of your review a trivial fix that we're hoping to land in the release? https://code.edge.launchpad.net/~michael.nelson/launchpad/db-611568-no-email-for-commercial-subscriptions/+merge/34757
[15:32] <leonardr> oh, right, it's tuesday
[15:32] <leonardr> mars, are you around? it's your turn
[15:35] <leonardr> noodles775, r=me
[15:36] <noodles775> Thanks leonardr !
[15:36] <noodles775> gmb: If you've time, the above MP is an RC.
[15:38] <gmb> noodles775, Sure, I'll take a look now.
[15:38] <henninge> noodles775: Hi!
[15:38] <noodles775> Hiya henninge
[15:39] <henninge> noodles775: I am sorry, I got busy doing other things ...
[15:39] <gmb> noodles775, rc=me.
[15:39] <noodles775> Thanks gmb
[15:39] <gmb> noodles775, That's bound for db-devel isn't it?
[15:39] <noodles775> gmb: I've set it for db-devel yes (in case devel is closed).
[15:39] <henninge> noodles775: http://launchpadlibrarian.net/55136007/distro-series-differences.png
[15:39] <gmb> noodles775, Brilliant. (it isn't closed but will be soon, hopfully)
[15:39] <henninge> noodles775: ^ that is what we already have?
[15:40] <noodles775> henninge: no, that's what that branch adds. It's the first step towards the planned UI.
[15:41] <henninge> noodles775: so the mockup is about what the planned UI is but it will be done in a future branch or branches?
[15:41] <henninge> noodles775: and the screenshot is what this review is currently about?
[15:42] <noodles775> henninge: yes, I need to do it in stages to keep it manageable. I'm not sure if you saw my comment on the MP, regarding whether I should wait until the UI is complete before requesting a review... I think we may need to re-think this with feature flags.
[15:42] <henninge> oh, missed that ;)
[15:43] <henninge> noodles775: no, UI reviews should be done as early as possible as they influence the coding
[15:43] <noodles775> I don't want to build up a huge branch (or pipeline of braches) locally, so landing the UI incrementally seemed sane.
[15:43] <noodles775> Yep, great.
[15:44] <henninge> noodles775: what happens if the "latest comment" is longer? Wrapping or truncating?
[15:45] <noodles775> Currently it would wrap.
[15:46] <henninge> noodles775: I think I'd prefer truncating ...
[15:46] <henninge> or wrap once, then truncate if that is not too hard.
[15:46] <henninge> since there will be an option to expand an entry, the user will have easy access to the full comment.
[15:48] <noodles775> henninge: yep, I agree (and yes, truncating won't stop wrapping, but it will be better than displaying long comments.)
[15:49] <henninge> noodles775: and maybe "no signer" should simply be "unknown"
[15:50] <henninge> noodles775: you know, I wish we had an easy way for "personal demos".
[15:50] <henninge> just push the branch and let it run.
[15:50] <henninge> Now I have to branch, make schema, make run to see your branch in action.
[15:51] <noodles775> henninge: I did try to make it easy with the script ;) (ie. merge;make schema;bin/iharness < sample_data.py)
[15:52] <noodles775> henninge: I think we can (or at least, could) put ec2 instances up for demo, but not sure its worth it.
[15:53] <henninge> noodles775: there was a description for that, iirc. Maybe there is already an "ec2" command for that.
[15:53]  * henninge has to explore that but not now.
[15:53] <henninge> noodles775: I am just wondering what the package name and versions link to.
[15:54] <henninge> noodles775: and what does the "-" link to in the first column?
[15:54] <henninge> it looks blue
[15:54] <noodles775> Both columns link to the source package publishing histories in the parent/derived series respectively.
[15:55] <noodles775> henninge: it is part of the same link. The first column uses the name-version as the link text, the second column doesn't repeat the name (as in the mockup, as having a separate column for the name looked strange).
[15:56] <noodles775> (which is why the column headers are "Warty package"/"Hoary version")
[15:56] <henninge> yes, that is quite a neat solution
[15:57] <henninge> I was just considering if having an extra column would really look that bad
[15:57] <noodles775> Regarding the "no signer", that was just based on the precedent of the PPA packages page, but unknown might be clearer.
[15:57] <henninge> but this solution is fine, too.
[15:57] <henninge> noodles775: also, the version numbers seem to go *down* from parent to child or is that just a glitch in your sample data?
[15:57] <henninge> noodles775: what is a blacklist, btw?
[15:58] <henninge> Argh!
[15:58] <noodles775> oh, you mean from the mockup (I was confused, knowing it wasn't mentioned on the screenshot).
[15:58] <noodles775> ?
[15:58] <henninge> noodles775: yes, mockup
[15:59] <henninge> Argh!
[15:59] <henninge> noodles775: I just had a call and have to run out right now. Terribly sorry.
[15:59] <henninge> noodles775: Is it OK if I sum up our conversation to a review later?
[16:00] <noodles775> henninge: of course!
[16:00] <henninge> Thanks!
[16:00] <noodles775> henninge: for your summary, the choice of "blacklist" is here https://lists.launchpad.net/launchpad-dev/msg04536.html
[16:00] <henninge> thanks
[16:12] <leonardr> noodles775, are you still in the queue?
[16:13] <noodles775> leonardr: I am, for a different branch: https://code.edge.launchpad.net/~michael.nelson/launchpad/distro-series-difference-browser2/+merge/34739
[16:13] <leonardr> noodles775: ok, mars is off today, so i'll take it in a bit
[16:14] <noodles775> leonardr: thanks!
[16:42] <noodles775> leonardr: I need to be off, do you mind doing the review via email?
[16:42] <leonardr> noodles775, sure
[17:52] <henninge> rockstar: ping
[17:52] <rockstar> henninge, pong
[17:52] <henninge> rockstar: Hi! ;-)
[17:52] <rockstar> henninge, how's things?
[17:52] <henninge> very good, thanks. I just did a UI review.
[17:52] <henninge> https://code.edge.launchpad.net/~michael.nelson/launchpad/distro-series-difference-browser2/+merge/34739
[17:53] <rockstar> henninge, great, I'll take a look.
[17:55] <rockstar> henninge, instead of "no signer" or "unknown" perhaps we shouldn't show it at all.
[17:56] <henninge> rockstar: I thought the "when" information was still of interest?
[17:56] <henninge> rockstar: maybe the column should be called "Last uploaded" and the content "18 minutes ago by Foo Bar"
[17:57] <rockstar> henninge, I was just about to suggest that exact thing.
[17:57] <henninge> or just "18 minutes ago"
[17:57] <henninge> do we have similar columns elsewhere on LP?
[17:57] <rockstar> henninge, basically, if the user can't do anything about it, we shouldn't show "unknown" because the user may think they're doing something wrong.
[17:57]  * rockstar JUST learned that in his UX class.
[17:57] <henninge> good point
[17:57] <henninge> ;)
[17:58] <rockstar> henninge, we have similar date columns, and I think we could/should adopt a similar pattern to what we're proposing here.
[17:58] <henninge> sounds good
[17:58] <rockstar> henninge, great.
[18:00] <henninge> rockstar: Also, I begin to wonder if the Name of the commenter should not be linkified, too, like the uploader.
[18:00] <henninge> but that would put more stuff in the comment colmun.
[18:01] <rockstar> henninge, yeah, and we should use a full display name.
[18:01] <rockstar> henninge, if we still limit it to two lines, that would still be okay.
[18:01] <henninge> rockstar: did you noticed that the name of the currently logged-in user (top right) is not the full name anymore?
[18:01] <henninge> I wonder why that was changed.
[18:02] <henninge> rockstar: for the comment column: I actually think it should be like the upload column:
[18:02] <rockstar> henninge, because full names aren't unique.
[18:02] <henninge> ah
[18:02] <henninge> "18 minutes ago by Mark Shuttleworth: I am working on this"
[18:03] <rockstar> henninge, yup. I think that's a good idea.
[18:07] <henninge> rockstar: are you going to add those suggestions to the MP or shall I do it?
[18:08] <rockstar> henninge, you could just put our whole discussion from IRC copy-n-pasted into a comment there.
[18:11] <henninge> rockstar: done
[19:43] <gary_poster> leonardr: up for a short but critical review?  https://code.edge.launchpad.net/~gary/launchpad/bug632218/+merge/34785 .  The description is a bit bare bones, but I'm hoping the linked bug report (bug 632218) gives you an idea of what is going on and what I set out to do.
[19:43] <_mup_> Bug #632218: staging builddmaster is having building woes with permissions - template engine <canonical-losa-lp> <Launchpad Foundations:Triaged by gary> <https://launchpad.net/bugs/632218>
[19:43] <leonardr> gary: ok, if it's shorti 'll take it now
[19:44] <gary_poster> it is
[19:55] <gary_poster> leonardr: (did you see that? ^^^)
[19:55] <leonardr> gary: yes, reading the bug now
[19:55] <gary_poster> cool thank you
[19:56] <leonardr> gary: so you remove *.pt.py during make clean?
[19:57] <gary_poster> yes, leonardr
[19:57] <leonardr> and you give the 'compile' step responsibility for (re-)creating them?
[19:57] <gary_poster> yes
[19:57] <leonardr> ok, r=me
[19:57] <gary_poster> thank you
[20:22] <bac> leonardr: have time for a modest branch to review?
[20:22] <leonardr> bac, sure, put it in the queue
[20:22] <bac> leonardr: not RC
[20:22] <bac> leonardr: https://code.edge.launchpad.net/~bac/launchpad/bug-237722/+merge/34790
[21:13] <benji> leonardr: given the queue and the time, I doubt you have time for any more reviewing today, right?
[21:13] <leonardr> benji: actually the queue is about to be emptied. i can do one more if it's not too long
[21:14] <benji> leonardr: it should be pretty short: https://code.edge.launchpad.net/~benji/launchpad/bug-413174/+merge/34794
[21:14] <benji> it's just using the new lazr.restful.error.expose functionality to kill a couple of OOPSes in LP
[21:16] <leonardr> cool
[21:22] <bac> thanks for the nice review leonardr
[21:36] <leonardr> benji, why put the message as the default in MultipleProductReleases? i'd rather see it at the single raise site
[21:36] <leonardr> in fact, why have a separate exception that you only use once?
[21:38] <benji> I'm not married to it.  My reasoning was that there is only one reason to raise that exception (vs. the other exception I added) so I made it the default text.  In other words, put it in the place that matches its generality.
[21:38] <leonardr> would it be make sense to raise expose(Exception("A milestone can only have...")) ?
[21:39] <leonardr> or ValueError or something
[21:39] <benji> re. separate exception: we want to be able to distinguish it from other exceptions that might be raised from that function
[21:39] <benji> right, I considered ValueError, but the cost of adding a more specific exception seemed lower than the value of being explicit
[21:43] <leonardr> ok, it's your call
[21:43] <leonardr> can you refactor get_last_oops_id into a helper module?
[21:46] <leonardr> benji, on line 246, if for some reason the exception isn't thrown, the test will pass when it should fail
[21:46] <leonardr> oh, no, nevermind
[21:46] <leonardr> you tested that immediately before
[21:58] <leonardr> benji: looks good, just refactor get_last_oops_id
[22:05] <james_w`> benji: you might like self.assertNoNewOops
[22:05] <benji> ooh, I might indeed
[22:05] <james_w`> currently only used in lib/lp/services/job/tests/test_runner.py
[22:06] <james_w`> plus, TestCases have an oopses attribute that might be useful
[22:14] <benji> leonardr: in what way do you want me to "[r]efactor get_last_oops_id"?
[22:15] <leonardr> let me double check, but i think you define the same method twice
[22:15] <benji> (I think I'll just be removing it.)
[22:15] <leonardr> ah, yes, just remove it
[22:15] <benji> leonardr: ah, indeed!
[22:15] <benji> thanks