| === rockstar changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | ||
| thumper | https://code.edge.launchpad.net/~thumper/lazr-js/multi-line-edit-class/+merge/13753 | 03:31 |
|---|---|---|
| thumper | https://code.launchpad.net/~thumper/launchpad/bmp-inline-commit-message/+merge/13814 for the bored | 04:06 |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | ||
| allenap | adeuring: Fancy a very short review? https://code.edge.launchpad.net/~allenap/launchpad/log-statement-none/+merge/13828 | 11:09 |
| adeuring | alsure | 11:09 |
| adeuring | allenap: yes | 11:09 |
| allenap | adeuring: Thanks. | 11:09 |
| adeuring | allenap: this look good, overall, so r=me, with one condition: From time to time it is useful to have all SQL statements logged. I think we have some script which directly use a the DB API module. So: could you add the "SET log_statement..." stuff to the debugging wiki page? | 11:17 |
| allenap | adeuring: Sure :) | 11:20 |
| allenap | Thanks! | 11:20 |
| === henninge_ is now known as henninge | ||
| henninge | adeuring: Hallo Abel! | 11:38 |
| adeuring | hi henninge! | 11:38 |
| henninge | adeuring: I have a shord little branch here for you. Can you please have a look at it? | 11:39 |
| henninge | https://code.edge.launchpad.net/~henninge/launchpad/bug-458924/+merge/13831 | 11:39 |
| henninge | short | 11:39 |
| adeuring | henninge: klar! | 11:39 |
| henninge | ;-) | 11:39 |
| adeuring | henninge: could you change the description of :param advance: in the __init__ doc string so that we know that the value N means N seconds? | 11:47 |
| henninge | adeuring: sure | 11:47 |
| adeuring | henninge: thanks. I am also wondering if we should add some tests. | 11:47 |
| henninge | adeuring: testing test code? | 11:48 |
| adeuring | henninge: sure ;) I have branch ready for review that allows to run docstring tests in this file. | 11:48 |
| henninge | adeuring: so add a docstring test? | 11:49 |
| henninge | adeuring: I thought we were removing these in-line doctests? | 11:49 |
| henninge | maybe it's different for test code. | 11:50 |
| adeuring | henninge: do we? I think the come very handy for these test helpers: YOu can see how they are working and that they are working | 11:50 |
| henninge | adeuring: there is lib/lp/testing/tests | 11:51 |
| henninge | adeuring: maybe we should add lib/lp/testing/doc ? | 11:51 |
| adeuring | henninge: OK, that's the other option. Would be fine for me. | 11:52 |
| henninge | adeuring: there is only one other docstring test in the file. | 11:53 |
| adeuring | henninge: but soon another one (assuming that the doc string test is accepted by a reviwerr): https://code.edge.launchpad.net/~adeuring/launchpad/bug-458160-fix-submissionparser-checkconsistency/+merge/13827 | 11:53 |
| adeuring | henninge: there is also code that invokes the doc string tests. lib/lp/testing/tests/test_inlinetests.py# | 11:54 |
| henninge | adeuring: ok, I'll add the docstring test because that is easier now. I may be wrong about docstring tests being deprecated but if you reviewer complains, let me know about it. | 11:56 |
| adeuring | henninge: OK. Or we both wait for barry to review our branches ? | 11:56 |
| henninge | adeuring: or that. | 11:57 |
| henninge | adeuring: I am in no hurry about this branch. | 11:57 |
| adeuring | henninge: OK, so let's wait ;) | 11:57 |
| henninge | adeuring: ok, I added the docstring test and ran it sucessfully using your branch. | 12:14 |
| henninge | pushing it now | 12:14 |
| adeuring | henninge: great! | 12:14 |
| === matsubara-afk is now known as matsubara | ||
| === mrevell is now known as mrevell-lunch | ||
| salgado | adeuring, looks like you need a review yourself? | 12:48 |
| adeuring | salgado: yes, as well as henninge. (his branch could benefit from the file lib/lp/testing/tests/test_inlinetests.py in my branch. which allows to run doc test strings in lp.testing) | 13:13 |
| leonardr | adeuring, would you take a look at https://code.edge.launchpad.net/~leonardr/launchpad/test-launchpadlib/+merge/13834 ? | 13:19 |
| adeuring | leonardr: sure | 13:19 |
| === mrevell-lunch is now known as mrevell | ||
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | ||
| adeuring | leonardr: r=me | 13:23 |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | ||
| salgado | adeuring, ok, I'll do yours | 13:24 |
| adeuring | salgado: thanks! | 13:24 |
| salgado | it's huge | 13:25 |
| salgado | adeuring, what diff should I review? the one in your cover letter or the one auto generated? | 13:26 |
| adeuring | salgado: the one in the cover letter. The auto generated one contains changes from a previous, not-yet-landed branch. | 13:27 |
| salgado | ok, cool | 13:27 |
| adeuring | (But the previous branch has benn reviewed) | 13:27 |
| leonardr | allenap, here's the merge proposal: https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/set-credentials/+merge/13837 | 14:35 |
| allenap | leonardr: Cool. | 14:35 |
| gmb | adeuring: Do you have time to review https://code.edge.launchpad.net/~gmb/launchpad/make-me-super-bug-438985/+merge/13840 for me? | 14:56 |
| adeuring | gmb: sure | 14:57 |
| gmb | adeuring: Thanks. | 15:06 |
| === ursula is now known as Ursinha | ||
| === sinzui changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ | ||
| sinzui | adeuring: do you have time for a branch that moves a lot of code to make two small UI changes? https://code.edge.launchpad.net/~sinzui/launchpad/package-link-validation-3/+merge/13844 | 15:44 |
| adeuring | sinzui: just started another review, but when that one is finished, sure | 15:45 |
| sinzui | beuno: Converting the pacakging Delete link button to an image was easy: http://people.canonical.com/~curtis/dsp-packaging.png | 15:49 |
| beuno | sinzui, neat | 15:50 |
| === salgado is now known as salgado-lunch | ||
| EdwinGrubbs | adeuring: I have an easy and fairly urgent mp: https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-458169-distroseries-timeout/+merge/13851 | 16:18 |
| bac | adeuring: scary trivial and not urgent at all: https://bugs.edge.launchpad.net/launchpad-registry/+bug/436508 | 16:19 |
| mup | Bug #436508: Incorrect capitalisation in "SSH Keys" on person index <post-3-ui-cleanup> <Launchpad Registry:In Progress by bac> <https://launchpad.net/bugs/436508> | 16:19 |
| adeuring | EdwinGrubbs: I'm currently review a branch from Curtis; might take some time... After this, taht's fine, but you also ask salgado-lunch or barry | 16:20 |
| adeuring | bac: ^^^ | 16:20 |
| bac | EdwinGrubbs: i'm on it! | 16:20 |
| EdwinGrubbs | bac: thanks | 16:20 |
| bac | EdwinGrubbs: trade? mine is really an rs | 16:20 |
| EdwinGrubbs | bac: sure | 16:25 |
| EdwinGrubbs | bac: r=me | 16:29 |
| bac | EdwinGrubbs: i get a test failure on productseries-views.txt | 16:33 |
| bac | let me make schema to ensure i have a clean starting point | 16:33 |
| EdwinGrubbs | that should fix it | 16:35 |
| bac | EdwinGrubbs: still failing | 16:38 |
| bac | can you re-run | 16:38 |
| bac | ./bin/test -vv -t 'distribution-soyuz.txt|productseries-views.txt' | 16:38 |
| bac | http://pastebin.ubuntu.com/299844/ | 16:38 |
| EdwinGrubbs | bac: it passed for me. Let me run make schema. | 16:48 |
| bac | EdwinGrubbs: hmm. karmic? | 16:48 |
| EdwinGrubbs | bac: I'm on karmic now | 16:48 |
| adeuring | sinzui: r=me | 16:48 |
| bac | EdwinGrubbs: shouldnt' make a difference. me too btw | 16:48 |
| sinzui | fab | 16:48 |
| leonardr | adeuring, your attention to https://code.edge.launchpad.net/~leonardr/launchpadlib/web-root/+merge/13856, please | 16:53 |
| adeuring | leonardr: sure | 16:53 |
| EdwinGrubbs | bac: it passed for me after running make schema. Oh, did you merge my branch into trunk. I haven't done that since yesterday. | 16:53 |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | ||
| bac | EdwinGrubbs: i did | 16:54 |
| EdwinGrubbs | bac: ok, it fails for me now. | 16:56 |
| bac | it looks like ProductSeriesUbuntuPackagingView has changed. i wonder if that was sinzui or thumper? | 16:56 |
| sinzui | me | 16:57 |
| sinzui | I updated the view to prevent it from creating duplicate packages. | 16:57 |
| === matsubara is now known as matsubara-lunch | ||
| bac | sinzui, EdwinGrubbs: so not it looks like it is having fits with the cached currentseries | 16:58 |
| sinzui | bac: EdwinGrubbs: I discovered that method while writing the test to verify that the view offers a new link when the current series changes. | 16:59 |
| === danilo__ is now known as danilos | ||
| EdwinGrubbs | bac, sinzui: oh, it's because the test is changing the status of a distroseries instead adding a new series, which clears the cache. | 17:02 |
| bac | EdwinGrubbs: yep | 17:02 |
| sinzui | right. | 17:02 |
| sinzui | EdwinGrubbs: This happen every 6 months when we switch the focus of development. The forms control the order that series progress through their status. Something cannot leap into a state | 17:04 |
| EdwinGrubbs | bac, sinzui: I could have the distroseries status setter call a new self.distribution.clearCache() method. Would that be too kludgy? | 17:04 |
| sinzui | EdwinGrubbs: no, that is the same pattern we use the preferredemail. | 17:05 |
| bac | EdwinGrubbs: are there other circumstances where the cache needs clearing? | 17:05 |
| === salgado-lunch is now known as salgado | ||
| EdwinGrubbs | sinzui, bac: Actually, I should just take off the cache on currentseries, since serieses is cached, currentseries will just loop through the list in memory, so that shouldn't be too expensive. | 17:09 |
| sinzui | EdwinGrubbs: is serieses sorted the same way? | 17:09 |
| * bac sends evil thoughts to whoever first used 'serieses' | 17:09 | |
| EdwinGrubbs | sinzui: serieses is sorted by the version attribute. | 17:11 |
| EdwinGrubbs | bac: don't you mean that you send evil thoughtses? | 17:11 |
| bac | eviles | 17:11 |
| sinzui | shut'Up! | 17:11 |
| bac | sinzui: was it you? | 17:12 |
| bac | no, it pre-dates all of us | 17:12 |
| * sinzui thought bac was doing a vicky quote | 17:12 | |
| bac | nah, i don't quote VP much. too much to type | 17:13 |
| adeuring | leonardr: could you add shorts for the new functions lookup_service_root() and loookup_web_root()? | 17:13 |
| adeuring | ...i mean shorts tests... | 17:13 |
| bac | sinzui: can i get an rs=sinzui for http://pastebin.ubuntu.com/299864/ | 17:15 |
| sinzui | Is that good grammar I see? | 17:16 |
| bac | real good | 17:16 |
| bac | i did good | 17:16 |
| adeuring | leonardr: what is the reason that you simply return service_root or web_root, if these avlues don't appear in the dictionaries? | 17:16 |
| sinzui | bac r=me | 17:16 |
| bac | sinzui: you serious? | 17:17 |
| sinzui | bacs good example has enbiggened all of us | 17:17 |
| sinzui | bac: yes | 17:17 |
| sinzui | there may be a bug for that | 17:18 |
| * bac runs the tests and takes cover under the sofa | 17:18 | |
| bac | EdwinGrubbs: in your branch could you make two small fixes to browser/productseries.py? | 17:20 |
| bac | 1) s/OBSOLETE"/OBSOLETE - the extra quote messes up emacs fiercely | 17:21 |
| bac | 2) s/staic/static | 17:21 |
| EdwinGrubbs | bac: I've pushed up a version with the productseries changes and I removed the currentseries cache, which makes the test pass. | 17:25 |
| bac | EdwinGrubbs: ok. i'll grab it | 17:26 |
| === beuno is now known as beuno-lunch | ||
| leonardr | adeuring: i do. if the value doesn't appear in the dictionary, it returns the 'default', which is service_root or web_root, depending | 17:32 |
| adeuring | leonardr: yes, but why? What is the reason for having "something" == lookup_web_root("something")? Wouldn't an exception make sense? | 17:35 |
| leonardr | adeuring: i'm sorry, i misunderstood your question | 17:37 |
| leonardr | an exception doesn't make sense because the most common value to fail a lookup is a URL | 17:37 |
| adeuring | leonardr: could you explain a bit? | 17:37 |
| bac | EdwinGrubbs: r=bac. thanks! | 17:38 |
| leonardr | sure, here's a test i just wrote | 17:38 |
| leonardr | self.assertEqual( | 17:38 |
| leonardr | uris.lookup_service_root("edge"), uris.service_roots['edge']) | 17:38 |
| leonardr | self.assertEqual( | 17:38 |
| leonardr | uris.lookup_service_root("http://some-other-server.com"), | 17:38 |
| leonardr | "http://some-other-server.com") | 17:38 |
| === gary_poster is now known as gary-lunch | ||
| adeuring | leonardr: Ah, I see. Nevertheless, I am a bit concerned about this: Let's assume that we have a LP client where you can choose the sever via an option, like "-s egde". If this is passed to lookup_web_root, the nonsensical value is simply returned. | 17:40 |
| leonardr | if you want me to make sure that the incoming string is either a valid alias or a url, i can do that | 17:40 |
| leonardr | it won't make much difference | 17:41 |
| adeuring | leonardr: yes, that would be great. I suspect that you'd have other wise to chase errors with bad URLs elsewhere in the a program that uses lplib | 17:41 |
| leonardr | adeuring, take another look | 17:56 |
| * adeuring is looking | 17:56 | |
| leonardr | adeuring: the diff doesn't show the difference, pull the branch | 17:58 |
| adeuring | leonardr: did that already ;) | 17:58 |
| leonardr | ok | 17:58 |
| adeuring | leonardr: thanks for the changes! r=me | 17:59 |
| leonardr | all right | 17:59 |
| === matsubara-lunch is now known as matsubara | ||
| === gary-lunch is now known as gary_poster | ||
| === adeuring changed the topic of #launchpad-reviews to: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ | ||
| === beuno-lunch is now known as beuno | ||
| deryck | rockstar, ping | 19:34 |
| rockstar | deryck, yo. | 19:35 |
| deryck | rockstar, hey, man. Could I ask a review of you? *Very* small Windmill test update. I just have a pastebin diff. | 19:37 |
| rockstar | deryck, I'm on the phone right now, but yes, I can get to it when I'm off. | 19:37 |
| deryck | rockstar, thanks. http://pastebin.ubuntu.com/299958/ | 19:37 |
| deryck | rockstar, fix for Bug #459144 | 19:38 |
| mup | Bug #459144: test_bug_privacy_settings failing in Windmill <Launchpad Bugs:In Progress by deryck> <https://launchpad.net/bugs/459144> | 19:38 |
| rockstar | deryck, is there an mp for it? | 20:11 |
| deryck | rockstar, no. if you want me to file one I can, but I branched from a large branch of windmill test moving around, so diff will be off. | 20:12 |
| rockstar | deryck, that's fine, since I have the pastebin. I just want somewhere to record my approve I guess. | 20:12 |
| deryck | rockstar, yeah, I prefer doing mp's, but in the case, it just seemed easy and out of mp | 20:13 |
| deryck | rockstar, so you're cool with the change then? | 20:20 |
| rockstar | deryck, yes. | 20:21 |
| deryck | rockstar, cool, thanks | 20:22 |
| abentley | flacoste: Thanks for your review. Could you also look at https://code.edge.launchpad.net/~abentley/launchpad/parse-binary/+merge/13600 ? | 21:10 |
| EdwinGrubbs | sidnei: ping | 21:11 |
| flacoste | abentley: done | 21:11 |
| EdwinGrubbs | :q | 21:11 |
| abentley | flacoste: Thanks again! | 21:12 |
| sidnei | EdwinGrubbs: aye | 21:20 |
| EdwinGrubbs | sidnei: I haven't run "make" on lazr-js in a long time, so I may be missing something. I get an error that it can't find a distribution for zc.recipe.testrunner. | 21:22 |
| sidnei | EdwinGrubbs: yeah, tis not in the download-cache. which begs the question: where is the download-cache for lazr-js supposed to be? | 21:22 |
| sidnei | EdwinGrubbs: i ran bin/buildout buildout:install-from-cache=false manually to get it downloaded | 21:23 |
| sidnei | EdwinGrubbs: i guess it's a question for BjornT really, since it's his branch that buildoutified it | 21:23 |
| === salgado is now known as salgado-afk | ||
| EdwinGrubbs | sidnei: now, I get this error: | 21:42 |
| EdwinGrubbs | File "bin/build", line 41, in <module> | 21:42 |
| EdwinGrubbs | import lazr.js.build | 21:42 |
| EdwinGrubbs | sidnei: is this just an intermediate branch, or should it be possible to get the examples to work? | 21:43 |
| sidnei | EdwinGrubbs: yeah, that's a problem from BjornT's branch too. if you remove any python-lazr* packages installed system-wide it should work | 21:43 |
| sidnei | EdwinGrubbs: i just managed to get the examples to work yes, if you have the latest rev. | 21:43 |
| EdwinGrubbs | sidnei: I got it to work. Is there a bug open for the conflicts it has with a locally installed lazr? | 22:02 |
| sidnei | EdwinGrubbs: there's some comments on the review for the buildoutification branch. i assume BjornT will fix that when monday comes, before he merges his branch. | 22:04 |
| EdwinGrubbs | sidnei: I also was curious if there was any current problem we are trying to solve by calling renderUI with Y.after(), or is this just a new standard for YUI3. | 22:05 |
| === matsubara is now known as matsubara-afk | ||
| sidnei | EdwinGrubbs: yes, there is. renderUI is called before the WidgetStdMod extension is initialized, and we were trying to override the WidgetStdMod.BODY. it's much easier and cleaner to use after and do it after it has been initialized. | 22:06 |
| EdwinGrubbs | ahhh | 22:06 |
| sidnei | EdwinGrubbs: also, if you do that, subclasses dont have to call superclass.<method>.apply(), apparently. | 22:07 |
| sidnei | EdwinGrubbs: since calling on .after() just chains the call to after the previous call, following the class hierarchy. | 22:08 |
| EdwinGrubbs | sidnei: r=me | 22:09 |
| sidnei | EdwinGrubbs: thank you! | 22:09 |
| sinzui | bac: did your distribution.series banch succeed? | 22:11 |
| bac | sinzui: which is that? | 22:12 |
| bac | sinzui: oh, my serieseses -> series branch? no i had some test failures in registry which i fixed | 22:12 |
| bac | sinzui: i was running the bugs tests to see how they fared | 22:13 |
| bac | sinzui: then i was going to wait for edwin's more important branch to land so there would be no conflicts | 22:13 |
| bac | sinzui: i'm still waiting on my branch from yesterday to land. i'm at #5 now | 22:15 |
| bac | sinzui: argh. i see there are three more CPs in PQM gumming up the works. | 22:16 |
| bac | sinzui: care to do a quick review of https://code.edge.launchpad.net/~bac/launchpad/bug-436259-add-member/+merge/13875 | 22:17 |
| sinzui | bac: I can do the review in 30 minutes. I need to deliver my daughter to soccer practice | 22:20 |
| bac | sinzui: per my whining above you'll see i'm in no rush. have fun. | 22:21 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
