=== rockstar changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
thumperhttps://code.launchpad.net/~thumper/launchpad/bmp-inline-commit-message/+merge/13814 for the bored04:06
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
allenapadeuring: Fancy a very short review? https://code.edge.launchpad.net/~allenap/launchpad/log-statement-none/+merge/1382811:09
adeuringallenap: yes11:09
allenapadeuring: Thanks.11:09
adeuringallenap: 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
allenapadeuring: Sure :)11:20
=== henninge_ is now known as henninge
henningeadeuring: Hallo Abel!11:38
adeuringhi henninge!11:38
henningeadeuring: I have a shord little branch here for you. Can you please have a look at it?11:39
adeuringhenninge: klar!11:39
adeuringhenninge: 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
henningeadeuring: sure11:47
adeuringhenninge: thanks. I am also wondering if we should add some tests.11:47
henningeadeuring: testing test code?11:48
adeuringhenninge: sure ;) I have branch ready for review that allows to run docstring tests in this file.11:48
henningeadeuring: so add a docstring test?11:49
henningeadeuring: I thought we were removing these in-line doctests?11:49
henningemaybe it's different for test code.11:50
adeuringhenninge: do we? I think the come very handy for these test helpers: YOu can see how they are working and that they are working11:50
henningeadeuring: there is lib/lp/testing/tests11:51
henningeadeuring: maybe we should add lib/lp/testing/doc ?11:51
adeuringhenninge: OK, that's the other option. Would be fine for me.11:52
henningeadeuring: there is only one other docstring test in the file.11:53
adeuringhenninge: 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/1382711:53
adeuringhenninge: there is also code that invokes the doc string tests. lib/lp/testing/tests/test_inlinetests.py#11:54
henningeadeuring: 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
adeuringhenninge: OK. Or we both wait for barry to review our branches ?11:56
henningeadeuring: or that.11:57
henningeadeuring: I am in no hurry about this branch.11:57
adeuringhenninge: OK, so let's wait ;)11:57
henningeadeuring: ok, I added the docstring test and ran it sucessfully using your branch.12:14
henningepushing it now12:14
adeuringhenninge: great!12:14
=== matsubara-afk is now known as matsubara
=== mrevell is now known as mrevell-lunch
salgadoadeuring, looks like you need a review yourself?12:48
adeuringsalgado: 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
leonardradeuring, would you take a look at https://code.edge.launchpad.net/~leonardr/launchpad/test-launchpadlib/+merge/13834 ?13:19
adeuringleonardr: sure13: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/
adeuringleonardr: r=me13:23
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/
salgadoadeuring, ok, I'll do yours13:24
adeuringsalgado: thanks!13:24
salgadoit's huge13:25
salgadoadeuring, what diff should I review?  the one in your cover letter or the one auto generated?13:26
adeuringsalgado: the one in the cover letter. The auto generated one contains changes from a previous, not-yet-landed branch.13:27
salgadook, cool13:27
adeuring(But the previous branch has benn reviewed)13:27
leonardrallenap, here's the merge proposal: https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/set-credentials/+merge/1383714:35
allenapleonardr: Cool.14:35
gmbadeuring: Do you have time to review https://code.edge.launchpad.net/~gmb/launchpad/make-me-super-bug-438985/+merge/13840 for me?14:56
adeuringgmb: sure14:57
gmbadeuring: 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/
sinzuiadeuring: 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/1384415:44
adeuringsinzui: just started another review, but when that one is finished, sure15:45
sinzuibeuno: Converting the pacakging Delete link button to an image was easy: http://people.canonical.com/~curtis/dsp-packaging.png15:49
beunosinzui, neat15:50
=== salgado is now known as salgado-lunch
EdwinGrubbsadeuring: I have an easy and fairly urgent mp: https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-458169-distroseries-timeout/+merge/1385116:18
bacadeuring: scary trivial and not urgent at all:  https://bugs.edge.launchpad.net/launchpad-registry/+bug/43650816:19
mupBug #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
adeuringEdwinGrubbs: I'm currently review a branch from Curtis; might take some time... After this, taht's fine, but you also ask salgado-lunch or barry16:20
adeuringbac: ^^^16:20
bacEdwinGrubbs: i'm on it!16:20
EdwinGrubbsbac: thanks16:20
bacEdwinGrubbs: trade?  mine is really an rs16:20
EdwinGrubbsbac: sure16:25
EdwinGrubbsbac: r=me16:29
bacEdwinGrubbs: i get a test failure on productseries-views.txt16:33
baclet me make schema to ensure i have a clean starting point16:33
EdwinGrubbsthat should fix it16:35
bacEdwinGrubbs: still failing16:38
baccan you re-run16:38
bac./bin/test -vv -t 'distribution-soyuz.txt|productseries-views.txt'16:38
EdwinGrubbsbac: it passed for me. Let me run make schema.16:48
bacEdwinGrubbs: hmm.  karmic?16:48
EdwinGrubbsbac: I'm on karmic now16:48
adeuringsinzui:  r=me16:48
bacEdwinGrubbs: shouldnt' make a difference.  me too btw16:48
leonardradeuring, your attention to https://code.edge.launchpad.net/~leonardr/launchpadlib/web-root/+merge/13856, please16:53
adeuringleonardr: sure16:53
EdwinGrubbsbac: 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/
bacEdwinGrubbs: i did16:54
EdwinGrubbsbac: ok, it fails for me now.16:56
bacit looks like ProductSeriesUbuntuPackagingView has changed.  i wonder if that was sinzui or thumper?16:56
sinzuiI updated the view to prevent it from creating duplicate packages.16:57
=== matsubara is now known as matsubara-lunch
bacsinzui, EdwinGrubbs: so not it looks like it is having fits with the cached currentseries16:58
sinzuibac: 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
EdwinGrubbsbac, 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
bacEdwinGrubbs: yep17:02
sinzuiEdwinGrubbs: 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 state17:04
EdwinGrubbsbac, sinzui: I could have the distroseries status setter call a new self.distribution.clearCache() method. Would that be too kludgy?17:04
sinzuiEdwinGrubbs: no, that is the same pattern we use the preferredemail.17:05
bacEdwinGrubbs: are there other circumstances where the cache needs clearing?17:05
=== salgado-lunch is now known as salgado
EdwinGrubbssinzui, 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
sinzuiEdwinGrubbs: is serieses sorted the same way?17:09
* bac sends evil thoughts to whoever first used 'serieses'17:09
EdwinGrubbssinzui: serieses is sorted by the version attribute.17:11
EdwinGrubbsbac: don't you mean that you send evil thoughtses?17:11
bacsinzui: was it you?17:12
bacno, it pre-dates all of us17:12
* sinzui thought bac was doing a vicky quote17:12
bacnah, i don't quote VP much.  too much to type17:13
adeuringleonardr: could you add shorts for the new functions lookup_service_root() and loookup_web_root()?17:13
adeuring...i mean shorts tests...17:13
bacsinzui: can i get an rs=sinzui for http://pastebin.ubuntu.com/299864/17:15
sinzuiIs that good grammar I see?17:16
bacreal good17:16
baci did good17:16
adeuringleonardr: what is the reason that you simply return service_root or web_root, if these avlues don't appear in the dictionaries?17:16
sinzuibac r=me17:16
bacsinzui: you serious?17:17
sinzuibacs good example has enbiggened all of us17:17
sinzuibac: yes17:17
sinzuithere may be a bug for that17:18
* bac runs the tests and takes cover under the sofa17:18
bacEdwinGrubbs: in your branch could you make two small fixes to browser/productseries.py?17:20
bac1) s/OBSOLETE"/OBSOLETE  - the extra quote messes up emacs fiercely17:21
bac2) s/staic/static17:21
EdwinGrubbsbac: I've pushed up a version with the productseries changes and I removed the currentseries cache, which makes the test pass.17:25
bacEdwinGrubbs: ok.  i'll grab it17:26
=== beuno is now known as beuno-lunch
leonardradeuring: i do. if the value doesn't appear in the dictionary, it returns the 'default', which is service_root or web_root, depending17:32
adeuringleonardr: yes, but why? What is the reason for having "something" == lookup_web_root("something")? Wouldn't an exception make sense?17:35
leonardradeuring: i'm sorry, i misunderstood your question17:37
leonardran exception doesn't make sense because the most common value to fail a lookup is a URL17:37
adeuringleonardr: could you explain a bit?17:37
bacEdwinGrubbs: r=bac.  thanks!17:38
leonardrsure, here's a test i just wrote17: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
adeuringleonardr: 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
leonardrif you want me to make sure that the incoming string is either a valid alias or a url, i can do that17:40
leonardrit won't make much difference17:41
adeuringleonardr: 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 lplib17:41
leonardradeuring, take another look17:56
* adeuring is looking17:56
leonardradeuring: the diff doesn't show the difference, pull the branch17:58
adeuringleonardr: did that already ;)17:58
adeuringleonardr: thanks for the changes! r=me17:59
leonardrall right17: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
deryckrockstar, ping19:34
rockstarderyck, yo.19:35
deryckrockstar, hey, man.  Could I ask a review of you?  *Very* small Windmill test update.  I just have a pastebin diff.19:37
rockstarderyck, I'm on the phone right now, but yes, I can get to it when I'm off.19:37
deryckrockstar, thanks.  http://pastebin.ubuntu.com/299958/19:37
deryckrockstar, fix for Bug #459144 19:38
mupBug #459144: test_bug_privacy_settings failing in Windmill <Launchpad Bugs:In Progress by deryck> <https://launchpad.net/bugs/459144>19:38
rockstarderyck, is there an mp for it?20:11
deryckrockstar, 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
rockstarderyck, that's fine, since I have the pastebin.  I just want somewhere to record my approve I guess.20:12
deryckrockstar, yeah, I prefer doing mp's, but in the case, it just seemed easy and out of mp20:13
deryckrockstar, so you're cool with the change then?20:20
rockstarderyck, yes.20:21
deryckrockstar, cool, thanks20:22
abentleyflacoste: Thanks for your review.  Could you also look at https://code.edge.launchpad.net/~abentley/launchpad/parse-binary/+merge/13600 ?21:10
EdwinGrubbssidnei: ping21:11
flacosteabentley: done21:11
abentleyflacoste: Thanks again!21:12
sidneiEdwinGrubbs: aye21:20
EdwinGrubbssidnei: 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
sidneiEdwinGrubbs: yeah, tis not in the download-cache. which begs the question: where is the download-cache for lazr-js supposed to be?21:22
sidneiEdwinGrubbs: i ran bin/buildout buildout:install-from-cache=false manually to get it downloaded21:23
sidneiEdwinGrubbs: i guess it's a question for BjornT really, since it's his branch that buildoutified it21:23
=== salgado is now known as salgado-afk
EdwinGrubbssidnei: now, I get this error: 21:42
EdwinGrubbs  File "bin/build", line 41, in <module>21:42
EdwinGrubbs    import lazr.js.build21:42
EdwinGrubbssidnei: is this just an intermediate branch, or should it be possible to get the examples to work?21:43
sidneiEdwinGrubbs: yeah, that's a problem from BjornT's branch too. if you remove any python-lazr* packages installed system-wide it should work21:43
sidneiEdwinGrubbs: i just managed to get the examples to work yes, if you have the latest rev.21:43
EdwinGrubbssidnei: I got it to work. Is there a bug open for the conflicts it has with a locally installed lazr?22:02
sidneiEdwinGrubbs: 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
EdwinGrubbssidnei: 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
sidneiEdwinGrubbs: 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
sidneiEdwinGrubbs: also, if you do that, subclasses dont have to call superclass.<method>.apply(), apparently.22:07
sidneiEdwinGrubbs: since calling on .after() just chains the call to after the previous call, following the class hierarchy.22:08
EdwinGrubbssidnei: r=me22:09
sidneiEdwinGrubbs: thank you!22:09
sinzuibac: did your distribution.series banch succeed?22:11
bacsinzui: which is that?22:12
bacsinzui: oh, my serieseses -> series branch?  no i had some test failures in registry which i fixed22:12
bacsinzui: i was running the bugs tests to see how they fared22:13
bacsinzui: then i was going to wait for edwin's more important branch to land so there would be no conflicts22:13
bacsinzui: i'm still waiting on my branch from yesterday to land.  i'm at #5 now22:15
bacsinzui: argh.  i see there are three more CPs in PQM gumming up the works.22:16
bacsinzui: care to do a quick review of https://code.edge.launchpad.net/~bac/launchpad/bug-436259-add-member/+merge/1387522:17
sinzuibac: I can do the review in 30 minutes. I need to deliver my daughter to soccer practice22:20
bacsinzui: 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!