| mwhudson | maxb: looks like 3 to me, i guess you'd like me to ec2 land them all? | 00:13 |
|---|---|---|
| maxb | it's 3 now :-) | 00:13 |
| maxb | And yes, please land any you approve, thanks | 00:14 |
| === matsubara is now known as matsubara-afk | ||
| === abentley changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === adiroiban changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [adiroiban(bug-525992)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| mwhudson | maxb: defeated by testfix | 04:37 |
| mwhudson | sigh | 04:39 |
| mwhudson | anyone here? | 04:39 |
| maxb | Thanks for trying, on the strength of that I can probably get them resubmitted direct-to-pqm later | 06:10 |
| mwhudson | i'm on it | 06:13 |
| stub | https://code.edge.launchpad.net/~stub/launchpad/pending-db-changes/+merge/23116 | 06:28 |
| stub | https://code.edge.launchpad.net/~stub/launchpad/testsuite/+merge/23282 | 06:28 |
| stub | The first is mainly deletions and improves staging rebuild for any reviewers interested in that. | 06:28 |
| stub | The second removes all the now unnecessary commit's from the testsuite factory | 06:29 |
| mwhudson | stub: woooooooo | 06:31 |
| mwhudson | stub: does your testsuite branch make the test suite notably faster? | 06:40 |
| stub | Dunno | 06:40 |
| stub | Total: 25933 tests, 0 failures, 0 errors in 181 minutes 57.297 seconds. | 06:41 |
| stub | Is that faster from ec2? | 06:41 |
| mwhudson | Total: 25929 tests, 0 failures, 0 errors in 229 minutes 18.851 seconds. | 06:42 |
| mwhudson | looks like a 'yes' | 06:42 |
| mwhudson | 1585+# Explicitly state 'no permissions on these objects' to silence | 06:50 |
| mwhudson | 1586+# security.py warnings. | 06:50 |
| mwhudson | yay! | 06:50 |
| mwhudson | stub: both done | 06:51 |
| stub | Ta | 06:53 |
| * stub waits for his system to stop thrashing so he can land. | 06:53 | |
| mwhudson | stub: why is https://code.edge.launchpad.net/~stub/launchpad/testsuite private? | 06:54 |
| stub | old, old branch | 06:54 |
| mwhudson | ah | 06:54 |
| mwhudson | want me to make it public? | 06:54 |
| stub | Sure. Or I will in a tick. | 06:54 |
| mwhudson | done | 06:55 |
| mwhudson | no i want to experiment and see how many tests fail when we don't reset sequences between tests | 06:56 |
| mwhudson | *now | 06:56 |
| mrevell | Hello! Anyone care to take a look at a tour update branch for me? https://code.edge.launchpad.net/~matthew.revell/launchpad/10.04-tour-updates/+merge/23245 | 10:49 |
| wgrant | mrevell: "Git, Subversion or CVS" <- no Mercurial? | 10:53 |
| mrevell | wgrant, Sorry, on the phone. Mercurial support is experiemental isn't it? | 10:58 |
| wgrant | mrevell: It's been on production since February, and it isn't indicated in the UI as being experimental. | 11:00 |
| mrevell | wgrant, Thanks for the heads-up. The code guys haven't mentioned it to me as being announceable. jelmer, is the Mercurial import stable enough to be publicly announced? | 11:02 |
| === sidnei changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [adiroiban(bug-525992), sidnei (bug-562173)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jelmer | mrevell: no, it's not ready to be announced yet | 11:39 |
| jelmer | mrevell: the launchpad side of it works well, but bzr-hg itself has a couple of open issues that affect most of the imports | 11:40 |
| === jelmer changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [adiroiban(bug-525992), sidnei (bug-562173), jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [adiroiban(bug-525992), sidnei (bug-562173), jelmer, wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | Oh, it's Tuesday, isn't it? | 12:03 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: - || queue: [adiroiban(bug-525992), sidnei (bug-562173), jelmer, wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: - || queue: [sidnei (bug-562173), jelmer, wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | Oh, HI adiroiban! I just commented on your proposal and danilo on the bug ;-) | 12:26 |
| danilos | adiroiban, I find the positioning horrible fwiw :) my suggestion is on the bug, but if you can come up with something better, I'd be happy to listen :) | 12:27 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: lunch || queue: [sidnei (bug-562173), jelmer, wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adiroiban | danilos: yes, it is not very visible. How about this: http://bayimg.com/LalBkAaco ? | 12:40 |
| danilos | adiroiban, I can live with that if you provide a help pop-up as well | 12:41 |
| adiroiban | danilos: hm... help popup for the first time a user click it? | 12:42 |
| danilos | adiroiban, no, next to it ("What's this?") | 12:42 |
| adiroiban | aha | 12:42 |
| adiroiban | ok | 12:42 |
| adiroiban | I'll do it | 12:42 |
| danilos | adiroiban, also, do not change the default for the page, because that will bring us some annoyed people | 12:42 |
| adiroiban | yes | 12:42 |
| adiroiban | the default is not changed | 12:43 |
| danilos | adiroiban, i.e. make it default to 'reviewer mode' | 12:43 |
| danilos | adiroiban, cool, thanks | 12:43 |
| danilos | adiroiban, also, it'd be good to check with our UI experts if green link is ok for this (it has immediate effect, but stores nothing in the DB; I am not exactly sure of the semantics, and it looks right, but please check with someone like noodles :) | 12:44 |
| adiroiban | danilos: should I leave the bootom notice „Working in translator mode” ? | 12:44 |
| danilos | adiroiban, I think it's useless to have it, and besides, nobody scrolls that far down anyway (except perhaps you :P) | 12:45 |
| adiroiban | danilos: sure, as soon as you are ok with it I will ask an UI expert :) | 12:45 |
| danilos | adiroiban, ideally, you'd be getting a completely different user interface (i.e. exactly what translators get today), but I am sure that's quite complex to implement considering how templates are crafted today | 12:45 |
| adiroiban | danilos: it's ok if this code will be someday no longer needed.. but until then, it should help a few reviewer | 12:47 |
| danilos | adiroiban, sure | 12:47 |
| === mrevell is now known as mrevell-lunch | ||
| noodles775 | Hi adiroiban, if the green link is just the .js-action class, then I think that's fine (eg. we use it already for expand/collapse sections that are only available via JS). | 13:16 |
| adiroiban | noodles775: hi. it is just a span, with cursor:pointer | 13:17 |
| adiroiban | it is not a real link | 13:17 |
| noodles775 | adiroiban: let one of us know when you're ready for a full UI review (in particular, rockstar and intellectronica are trying to build up their number of UI reviews). | 13:18 |
| noodles775 | adiroiban: Just like an expand/collapse segment? | 13:18 |
| * noodles775 finds one. | 13:19 | |
| adiroiban | clicking on it will just set a cookie | 13:19 |
| noodles775 | adiroiban: oh, it doesn't update the page at all indicating the mode switch? | 13:20 |
| * noodles775 hasn't read the bug. | 13:20 | |
| adiroiban | noodles775: http://bayimg.com/nAlBfaAcO | 13:20 |
| adiroiban | this is the initial page | 13:20 |
| adiroiban | and after clickin on it | 13:20 |
| adiroiban | it will turn http://bayimg.com/NALBHAACo | 13:21 |
| === henninge changed the topic of #launchpad-reviews to: On call: - || reviewing: lunch || queue: [sidnei (bug-562173), jelmer, wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| * noodles775 is glad the 'What's this?' is there, as that's exacly what he'd be clicking on ;) | 13:22 | |
| noodles775 | adiroiban: so, assuming it's a js function, you should simply use the js-action class (which will style it green automatically). But grab one of the guys above for a full review when you're ready. | 13:25 |
| adiroiban | noodles775: ok. thanks | 13:25 |
| === mrevell-lunch is now known as mrevell | ||
| === henninge changed the topic of #launchpad-reviews to: On call: - || reviewing: sidnei || queue: [jelmer, wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: sidnei || queue: [jelmer, wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sidnei | heya henninge | 13:31 |
| henninge | sidnei: Hi! just a quick question because I am not too sure about YUI | 13:31 |
| sidnei | shoot | 13:32 |
| henninge | sidnei: does "one" really work with queries that return more than one node? | 13:32 |
| * henninge is probably just thinking of storm and database queries ... | 13:33 | |
| sidnei | henninge, one returns one node, all returns a nodelist. not sure what happens if you use one with a selector that matches more than one node. i suspect it returns the first one | 13:33 |
| henninge | I hope it does ;-) | 13:33 |
| henninge | sidnei: but I guess transforming query to one is a recommended migration path? | 13:34 |
| sidnei | henninge, yes, query was renamed to one. queryAll was renamed to all | 13:34 |
| sidnei | henninge, for reference: https://pastebin.canonical.com/30484/ | 13:35 |
| henninge | ah, just a renaming. That's easy then ... ;-) | 13:35 |
| henninge | cool | 13:35 |
| henninge | sidnei: r=me | 13:35 |
| sidnei | henninge, thanks! | 13:35 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: jelmer || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | jelmer: which of the two reviews you have there do you want to have done (first) ? | 13:39 |
| henninge | oh, one is ovesized | 13:39 |
| henninge | ;) | 13:39 |
| jelmer | henninge: :-) | 13:41 |
| jelmer | henninge: The process-accepted-robust one please | 13:41 |
| jelmer | henninge: that's the more important one at least, though it would be nice to get the other one reviewed too at some point :-) | 13:41 |
| henninge | jelmer: I am just looking at the shorter one and see that it needs stormifying ... ;-) | 13:42 |
| henninge | jelmer: also please see the style guide about truth conditionals | 13:43 |
| henninge | https://dev.launchpad.net/PythonStyleGuide#Truth%20conditionals | 13:43 |
| jelmer | henninge: oh, that's yet another one I think.. | 13:45 |
| jelmer | henninge: thanks | 13:45 |
| henninge | jelmer: I added a comment https://code.edge.launchpad.net/~jelmer/launchpad/bug113563/+merge/23232 | 13:46 |
| henninge | jelmer: on to the big one. | 13:46 |
| jelmer | henninge: is that the process-accepted-robust one? | 13:47 |
| henninge | jelmer: yeah, just realized that's not it ... | 13:47 |
| henninge | where is the merge proposal? Is it not for Launchpad? | 13:47 |
| henninge | jelmer: ah, found it. At the bottom | 13:48 |
| henninge | jelmer: is there a bug for this one? No branch without a bug! ;) | 13:50 |
| henninge | needed for qa now | 13:50 |
| jelmer | there is a related critical bug (about one item failing and breaking process-accepted completely) but not about this specific issue I think; I'll file one | 13:51 |
| henninge | well, if it fixes the critical bug .... | 13:51 |
| henninge | just link it | 13:51 |
| henninge | jelmer: ^ | 13:51 |
| jelmer | henninge: ok, will do | 13:52 |
| henninge | jelmer: what's special about the "ubuntutest" distribution? | 13:59 |
| jelmer | henninge: not much, it's what's usually used for testing in soyuz | 14:00 |
| jelmer | henninge: and we need to use the same distribution anywhere in those tests, hence my changing it | 14:00 |
| henninge | jelmer: you mean in the whole TestCase or just the one testRobustness method? | 14:01 |
| henninge | using the same distribution | 14:02 |
| jelmer | henninge: in the whole testRobustness method we need to use the same distribution, but it shares a utility function with the other test on that testcase so I just opted to change that as well | 14:02 |
| jelmer | alternatively I could've modified it to take a distribution argument and pass in a different value in each test | 14:02 |
| henninge | jelmer: the reason behind my questions is that we try to cut down on the usage of sample data in the test and rather create what we need right there. | 14:03 |
| jelmer | henninge: ah, ok | 14:03 |
| henninge | if there is nothing special about "ubuntutest", you might as well just create a distribution from the factory on the spot | 14:04 |
| henninge | or in the setUp | 14:04 |
| gmb | henninge, Can I stick https://code.edge.launchpad.net/~gmb/launchpad/show-bwa-on-bw-pages-bug-558409 on your queue for a code review please? | 14:04 |
| henninge | gmb: you can, Brad should be around soon, too, so it should get picked up. | 14:05 |
| gmb | henninge, Ah, of course. Thanks. | 14:05 |
| === gmb changed the topic of #launchpad-reviews to: On call: henninge || reviewing: jelmer || queue: [wgrant, gmb] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| gmb | intellectronica, mrevell: Could I trouble you two gents, as UI reviewers, to take a squizz at https://code.edge.launchpad.net/~gmb/launchpad/show-bwa-on-bw-pages-bug-558409/+merge/23308? | 14:06 |
| mrevell | gmb, intellectronica is probably the better person to look right now as I've been out of the UI review loop for a few weeks. | 14:06 |
| intellectronica | gmb: sure thing. i'll ask sinzui to mentor the review too | 14:06 |
| gmb | mrevell, Fair enough. | 14:07 |
| gmb | intellectronica, Thanks. I'll request reviews on the MP. | 14:07 |
| intellectronica | gmb: any chance of screenshots? | 14:07 |
| gmb | intellectronica, Sure. Coming right up. | 14:08 |
| intellectronica | thanks | 14:08 |
| jelmer | henninge: I'll see if that's possible, though I suspect the SoyuzTestPublisher still relies on sampledata too much | 14:08 |
| henninge | jelmer: ah, so ubuntutest *is* special ... ;-) | 14:08 |
| henninge | jelmer: in any case, you should just do that in setUp, either get ubuntutest or create one from the factory. | 14:10 |
| jelmer | henninge: ok | 14:11 |
| henninge | jelmer: if it is too much hassle to use one from the factory, go with ubuntutest but don't repeat it throughout the tests. | 14:11 |
| * gmb hand-cranks launchpad.dev | 14:12 | |
| gmb | intellectronica, http://devpad.canonical.com/~gbinns/bwa-screenshot-1.png and http://devpad.canonical.com/~gbinns/bwa-screenshot-2.png | 14:13 |
| intellectronica | gmb: lovely, thanks | 14:14 |
| intellectronica | gmb: i think it can be improved by highlighting the last (current) status, but since this design is identical to the code imports ui, i'm not sure if we should change it at all | 14:17 |
| gmb | intellectronica, Yeah, I think that the general thinking is that we should keep the same design. | 14:17 |
| intellectronica | gmb: did you discuss that already with anyone before doing it? | 14:17 |
| gmb | intellectronica, When I talked to deryck yesterday he mentioned that someone (jml, I think) was keen to see the code design re-used, and this has been mentioned in previous discussions. | 14:18 |
| intellectronica | gmb: yes, i think it makes sense, and in that case, assuming we're not going to change the design on code pages too, i agree that leaving it like this is the pragmatic choice. | 14:19 |
| gmb | Cool | 14:19 |
| intellectronica | i still think it's worth finding a way of highlighting the last activity (both in code and in bugs). maybe later, though | 14:19 |
| intellectronica | gmb: so ui*=me. let's see what sinzui thinks | 14:20 |
| gmb | intellectronica, RIghto, thanks. | 14:20 |
| * sinzui is pulling the branch now | 14:21 | |
| jml | gmb: yeah, it was me. | 14:28 |
| gmb | jml, FTR, I was already planning to steal their design ;) | 14:28 |
| jml | gmb: good good :) | 14:29 |
| === bac changed the topic of #launchpad-reviews to: On call: henninge, bac || reviewing: jelmer || queue: [wgrant, gmb] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === bac changed the topic of #launchpad-reviews to: On call: henninge, bac || reviewing: jelmer,wgrant || queue: [gmb] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | jelmer: review sent | 14:47 |
| jelmer | henninge: thanks | 14:48 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge, bac || reviewing: -,wgrant || queue: [gmb] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| bac | hi wgrant -- you still around? | 15:02 |
| wgrant | bac: Just. | 15:03 |
| bac | wgrant: i'll make this snappy then | 15:04 |
| bac | i'm looking at your buildsequencer deletion branch | 15:04 |
| wgrant | Yep, thanks. | 15:04 |
| bac | i see there are still a lot of symbols in the config files. shouldn't they go away too? | 15:04 |
| wgrant | bac: See the commit message -- we can't remove them until they are removed from the production configs. | 15:05 |
| wgrant | (yes, forgot to note that in the description...) | 15:05 |
| bac | wgrant: ok. yeah, that's pretty subtle... :) | 15:06 |
| bac | wgrant: r=bac and i'll land it for you. have a good evening | 15:06 |
| wgrant | bac: Thanks. | 15:06 |
| jelmer | bac: Hi | 15:06 |
| jelmer | bac: can I add another review request to your queue? | 15:06 |
| bac | jelmer: yes, of course | 15:07 |
| jelmer | https://code.edge.launchpad.net/~jelmer/launchpad/refactor-permissions/+merge/23216 | 15:07 |
| === jelmer changed the topic of #launchpad-reviews to: On call: henninge, bac || reviewing: -,wgrant || queue: [gmb, jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === henninge changed the topic of #launchpad-reviews to: On call: bac || reviewing: wgrant || queue: [gmb, jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | bac: I have to bow out already. See you tomorrow! | 15:10 |
| bac | henninge: ok, ttyl | 15:10 |
| === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: gmb || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | gmb, ping | 15:14 |
| gmb | sinzui, Hi | 15:15 |
| sinzui | gmb, the bugwatch need a BreadCrumb adapter | 15:15 |
| sinzui | is there a bug reported for that? | 15:15 |
| * sinzui is searching now | 15:15 | |
| gmb | sinzui, Not AFAIK. | 15:15 |
| sinzui | gmb: I think your change is fine, but we need bug reporting that the page is lost since it has no heading or breadcrumbs | 15:16 |
| gmb | sinzui, Sure, I'll file one. | 15:17 |
| sinzui | gmb: I do not think they get editing very often since I do not see bugs about it | 15:18 |
| gmb | sinzui, They don't, except by us. | 15:18 |
| sinzui | oh good | 15:19 |
| sinzui | gmb, ui=me. I'll note we agreed to report a bug about the missing heading and breadcrumbs...there is no obvious parent for this | 15:20 |
| gmb | sinzui, Thanks. | 15:20 |
| === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: jelmer || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| gmb | bac, Thanks for the review. | 15:32 |
| bac | gmb: have you put tuaw.com on your RSS reader now? :) i only kid b/c i'm jealous. | 15:42 |
| gmb | bac, Nope, I spotted the news on Gizmodo. Shame the 13" isn't getting an i5, but I'm more concerned about RAM and HDD space than I am about processor speed. | 15:43 |
| === Ursinha_ is now known as Ursinha-lunch | ||
| bac | hi jelmer | 16:06 |
| jelmer | hi bac | 16:09 |
| bac | jelmer: could you merge your branch and fix the conflict? i'm getting it when i try to merge though it doesn't show on the MP | 16:10 |
| === deryck is now known as deryck[lunch] | ||
| jelmer | bac: okay | 16:11 |
| bac | thx | 16:11 |
| jelmer | bac: I also have another branch up for review, https://code.edge.launchpad.net/~jelmer/launchpad/oops-on-pool-overwrite-error/+merge/23328 | 16:12 |
| jelmer | which is a bit smaller | 16:12 |
| bac | jelmer: ok | 16:12 |
| === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: jelmer || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: jelmer || queue: [jelmer-oops] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: jelmer-permissions || queue: [jelmer-oops] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === barry is now known as flufl | ||
| === flufl is now known as barry | ||
| leonardr | allenap, can i get you to take one more look at https://code.edge.launchpad.net/~leonardr/lazr.restful/cache-service-root/+merge/22948 ? | 16:26 |
| leonardr | i'd also like you to review the corresponding lazr.restfulclient branch if you have time | 16:26 |
| allenap | leonardr: Sure. | 16:26 |
| leonardr | so i can get one person to look over the whole system | 16:26 |
| leonardr | i'm working on the l.rc branch now | 16:26 |
| allenap | leonardr: Cool, I'll do it when it's ready. I'm OCR tomorrow anyway. | 16:27 |
| leonardr | should just be a few minutes ,i just have to write the mp | 16:27 |
| leonardr | actually i already got it reviewed, but maybe you want to take a quick look | 16:28 |
| leonardr | https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/httplib-97-workaround/+merge/23263 | 16:29 |
| allenap | leonardr: What does the .replace("\xc3\xa7", "c") bit do? | 16:33 |
| leonardr | allenap: it's a mistake that i removed | 16:33 |
| leonardr | i added it a while back to get a test failure to show up | 16:33 |
| leonardr | i thought i pushed that update | 16:34 |
| leonardr | give me 1 minute, i'm testing a slight refactoring and i'll push that too | 16:34 |
| === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: jelmer-oops || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | leonardr: Ah, yes, you did; I was only looking at r130. | 16:34 |
| leonardr | allenap, slight refactoring has been pushed (r132) | 16:35 |
| allenap | leonardr: In the restfulclient branch, for the original request to be a conditional request wouldn't httplib2 have to have the representation cached already, and so return the cached version? | 16:44 |
| leonardr | allenap: yes, the problem is there's a bug in httplib2 where the cached version is cached incorrectly | 16:45 |
| leonardr | it returns the cached version and lazr.restfulclient chokes on it | 16:45 |
| allenap | leonardr: Is that different to #97, or am I misremembering it? | 16:48 |
| leonardr | allenap, that is #97 | 16:49 |
| allenap | leonardr: Okay. | 16:49 |
| === deryck[lunch] is now known as deryck | ||
| adiroiban | rockstar, intellectronica do you have time for an UI review https://code.edge.launchpad.net/~adiroiban/launchpad/bug-525992/+merge/23285 ? | 17:08 |
| rockstar | adiroiban, sure | 17:08 |
| adiroiban | rockstar: I have added links to some screenshots in the last comment | 17:09 |
| rockstar | adiroiban, okay. | 17:09 |
| bac | hi jelmer | 17:14 |
| jelmer | bac: hi | 17:14 |
| bac | jelmer i'm looking at your oops branch. it looks good but i have a question | 17:15 |
| bac | jelmer: at line 70 of the diff you remove a log message and just let the exception bubble up. | 17:16 |
| bac | jelmer: why did you determine that error message was of no use? | 17:16 |
| jelmer | bac: the error message is still of use, but it's logged in the OOPS now | 17:16 |
| jelmer | as well as by the caller of that method | 17:16 |
| jelmer | previously the function itself would log the error and then re-raise the exception to its caller, which would ignore it and not mark the record it was processing as published | 17:17 |
| jelmer | Sorry, my MP description was a bit terse | 17:18 |
| bac | jelmer: i'm confused b/c one change is to ArchivePublisherBase and the other to FilePublishingBase and they seem to not be related. is that right? | 17:21 |
| bac | so the caller to the method on FilePB is the one that is already handling the OOPS and has always done so? | 17:22 |
| jelmer | bac: they are somewhat related. ArchivePublisherBase.publish calls publish() on the elements in its "files" member that are derived from FilePublishingBase | 17:22 |
| jelmer | bac: yep | 17:23 |
| bac | jelmer: please consider using abentley's very nice bzr plugin lp-send to creating MPs. the skeleton makes for much better, more informative, easier to review merge proposals. | 17:23 |
| bac | jelmer: ok. thanks for the info | 17:23 |
| === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: lunch || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| abentley | jelmer, actually the lpreview_body plugin will work with "bzr send" and "bzr lp-propose" (recommended). | 17:25 |
| jelmer | bac: Thanks! | 17:25 |
| rockstar | adiroiban, you'll need to fix the jslint issues before you can land this. | 17:26 |
| adiroiban | rockstar: ok. Do I a need a documentation review from mrevell, or the help page is ok? | 17:28 |
| rockstar | adiroiban, you should have him look at it. | 17:28 |
| jelmer | abentley: ah, ok | 17:29 |
| rockstar | adiroiban, when one enters "translator mode," why is "Someone should review this translation" not checked? | 17:31 |
| adiroiban | rockstar: do you mean to check "Someone should ..." for the current new translations ? | 17:32 |
| adiroiban | I mean for the previouly entered translations? | 17:33 |
| rockstar | adiroiban, if I understand this correctly, when a translator is also a reviewer but just trying to translate, they always check "Someone should review this translation" Correct? | 17:33 |
| rockstar | adiroiban, and so this branch makes it so that they don't have to check that when they are in translation mode. | 17:34 |
| adiroiban | rockstar: that is correct | 17:34 |
| rockstar | adiroiban, so, when someone enters "translator mode" there's a cookie set using javascript, correct? | 17:35 |
| adiroiban | yes | 17:35 |
| rockstar | adiroiban, so you should also use javascript to check the box next to "Someone should review this translation" | 17:35 |
| adiroiban | hmm.. there is a javascript for that | 17:36 |
| adiroiban | is it not working? | 17:37 |
| adiroiban | I have tested it on Karmic using Chromium and Firefox | 17:37 |
| adiroiban | and there is also a Windmill test for that | 17:37 |
| rockstar | adiroiban, well, this is based on your screenshots. I'm pulling your branch now. | 17:37 |
| adiroiban | rockstar: ah. ok | 17:37 |
| adiroiban | yes | 17:38 |
| === Ursinha-lunch is now known as Ursinha | ||
| rockstar | adiroiban, I'm not really looking at your code specifically. You'll need to get a code review for that. All I'm looking at is the UI currently. | 17:38 |
| adiroiban | rockstar: yes. Henning will do the code review | 17:38 |
| rockstar | adiroiban, so, your current screenshots are wrong. noodles will probably pull the branch as well when he mentors my review, but you might want to update those, since they indicate something incorrect. | 17:40 |
| adiroiban | rockstar: why are they wrong ? :) | 17:41 |
| rockstar | adiroiban, because when you're in "translator mode," then "Someone should review this translation" should be checked. | 17:43 |
| adiroiban | rockstar: ah... not always | 17:43 |
| adiroiban | you can still manually uncheck them | 17:43 |
| adiroiban | is just when you type a new translation, the checkbox will be checked | 17:44 |
| rockstar | adiroiban, when you make screenshots, they should indicate the default. | 17:46 |
| adiroiban | rockstar: ok. I have updated the screenshot from the description | 17:50 |
| === leonardr is now known as leonardr-afk | ||
| === gary_poster is now known as gary-lunch | ||
| === mobile_ is now known as bac_ | ||
| === leonardr-afk is now known as leonardr | ||
| === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === gary-lunch is now known as gary_poster | ||
| gary_poster | abentley: can you give me a review of the testfix please? http://pastebin.ubuntu.com/413823/ | 20:14 |
| abentley | gary_poster, r=me. | 20:15 |
| gary_poster | thanks abentley | 20:15 |
| jml | hello | 20:26 |
| jml | may I please get a review of some fairly simple (and hopefully non-behavioural) changes to the codehosting ssh server? | 20:27 |
| jml | https://code.edge.launchpad.net/~jml/launchpad/extract-ssh-server/+merge/23350 | 20:27 |
| jml | also, https://code.edge.launchpad.net/~jml/launchpad/subunit-by-default/+merge/18449 | 20:39 |
| jml | (wtf, there have been ~4000 merge proposals since then?) | 20:39 |
| === EdwinGrubbs is now known as Edwin-afk | ||
| === matsubara is now known as matsubara-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
