| 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:32 |
|---|---|---|
| lifeless | http://bazaar.launchpad.net/~lifeless/launchpad/private-librarian/revision/9600 | 03:33 |
| lifeless | thumper: ^ | 03:33 |
| lifeless | thumper: hi ? | 04:17 |
| 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:18 |
| 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:19 |
| 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:20 |
| 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:21 |
| 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:22 |
| 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:23 |
| mwhudson | thumper: :( | 04:24 |
| mwhudson | thumper: which bit isn't working? | 04:24 |
| 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:25 |
| 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:26 |
| thumper | I found the problem | 04:45 |
| thumper | the tests were there | 04:45 |
| thumper | but not checking for the right thing | 04:45 |
| thumper | any one on call? | 11:07 |
| thumper | https://code.edge.launchpad.net/~thumper/launchpad/xmlrpc-tests/+merge/34719 | 11:08 |
| thumper | rollout blocker ^^^ | 11:08 |
| thumper | done by gmb | 11:20 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM; for RC reviews please add him as a reviewer | ||
| 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:26 |
| 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:27 |
| noodles775 | Thanks henninge | 12:28 |
| * noodles775 takes a break for lunch. | 12:28 | |
| === mrevell is now known as mrevell-lunch | ||
| jelmer | bac: Hi | 14:44 |
| bac | hi jelmer | 14:44 |
| 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:46 |
| bac | jelmer: ok. | 14:47 |
| bac | jelmer: you have gotten all of the tests to work now? | 14:48 |
| 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:49 |
| 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. | 14:50 |
| === mrevell-lunch is now known as mrevell | ||
| 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:06 |
| * bac was going to say "Imprimatur" but couldn't spell it. | 15:07 | |
| bac | gmb: actually, wait a bit on that. my testing may have been flawed | 15:08 |
| * gmb waits | 15:09 | |
| bac | gmb: done. | 15:18 |
| gmb | bac, Looking now. | 15:18 |
| 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:21 |
| 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:31 |
| leonardr | oh, right, it's tuesday | 15:32 |
| leonardr | mars, are you around? it's your turn | 15:32 |
| leonardr | noodles775, r=me | 15:35 |
| noodles775 | Thanks leonardr ! | 15:36 |
| noodles775 | gmb: If you've time, the above MP is an RC. | 15:36 |
| gmb | noodles775, Sure, I'll take a look now. | 15:38 |
| henninge | noodles775: Hi! | 15:38 |
| noodles775 | Hiya henninge | 15:38 |
| 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:39 |
| noodles775 | henninge: no, that's what that branch adds. It's the first step towards the planned UI. | 15:40 |
| 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:41 |
| 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:42 |
| 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:43 |
| henninge | noodles775: what happens if the "latest comment" is longer? Wrapping or truncating? | 15:44 |
| noodles775 | Currently it would wrap. | 15:45 |
| 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:46 |
| noodles775 | henninge: yep, I agree (and yes, truncating won't stop wrapping, but it will be better than displaying long comments.) | 15:48 |
| henninge | noodles775: and maybe "no signer" should simply be "unknown" | 15:49 |
| 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:50 |
| noodles775 | henninge: I did try to make it easy with the script ;) (ie. merge;make schema;bin/iharness < sample_data.py) | 15:51 |
| noodles775 | henninge: I think we can (or at least, could) put ec2 instances up for demo, but not sure its worth it. | 15:52 |
| 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:53 |
| 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:54 |
| 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:55 |
| noodles775 | (which is why the column headers are "Warty package"/"Hoary version") | 15:56 |
| henninge | yes, that is quite a neat solution | 15:56 |
| 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:57 |
| 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:58 |
| 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? | 15:59 |
| 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:00 |
| === leonardr changed the topic of #launchpad-reviews to: On call: leonardr || Reviewing: - || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM; for RC reviews please add him as a reviewer | ||
| leonardr | noodles775, are you still in the queue? | 16:12 |
| 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:13 |
| noodles775 | leonardr: thanks! | 16:14 |
| noodles775 | leonardr: I need to be off, do you mind doing the review via email? | 16:42 |
| leonardr | noodles775, sure | 16:42 |
| === benji is now known as benji-lunch | ||
| 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:52 |
| rockstar | henninge, great, I'll take a look. | 17:53 |
| rockstar | henninge, instead of "no signer" or "unknown" perhaps we shouldn't show it at all. | 17:55 |
| 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:56 |
| 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:57 |
| 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. | 17:58 |
| 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:00 |
| 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:01 |
| 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:02 |
| rockstar | henninge, yup. I think that's a good idea. | 18:03 |
| === benji-lunch is now known as benji | ||
| === deryck is now known as deryck[lunch] | ||
| henninge | rockstar: are you going to add those suggestions to the MP or shall I do it? | 18:07 |
| rockstar | henninge, you could just put our whole discussion from IRC copy-n-pasted into a comment there. | 18:08 |
| henninge | rockstar: done | 18:11 |
| === deryck[lunch] is now known as deryck | ||
| 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:43 |
| gary_poster | it is | 19:44 |
| gary_poster | leonardr: (did you see that? ^^^) | 19:55 |
| leonardr | gary: yes, reading the bug now | 19:55 |
| gary_poster | cool thank you | 19:55 |
| leonardr | gary: so you remove *.pt.py during make clean? | 19:56 |
| 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 | 19:57 |
| 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 changed the topic of #launchpad-reviews to: On call: leonardr || Reviewing: - || queue: [noodles775, bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM; for RC reviews please add him as a reviewer | ||
| bac | leonardr: https://code.edge.launchpad.net/~bac/launchpad/bug-237722/+merge/34790 | 20:22 |
| 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:13 |
| === leonardr changed the topic of #launchpad-reviews to: On call: leonardr || Reviewing: bac || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM; for RC reviews please add him as a reviewer | ||
| 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:14 |
| leonardr | cool | 21:16 |
| bac | thanks for the nice review leonardr | 21:22 |
| 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:36 |
| 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:38 |
| 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:39 |
| leonardr | ok, it's your call | 21:43 |
| leonardr | can you refactor get_last_oops_id into a helper module? | 21:43 |
| 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:46 |
| leonardr | benji: looks good, just refactor get_last_oops_id | 21:58 |
| 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:05 |
| james_w` | plus, TestCases have an oopses attribute that might be useful | 22:06 |
| === abentley_ is now known as abentley | ||
| benji | leonardr: in what way do you want me to "[r]efactor get_last_oops_id"? | 22:14 |
| 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 | 22:15 |
| === leonardr changed the topic of #launchpad-reviews to: On call: leonardr || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews || gmb is RM; for RC reviews please add him as a reviewer | ||
| === leonardr 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 || gmb is RM; for RC reviews please add him as a reviewer | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
