/srv/irclogs.ubuntu.com/2010/01/05/#launchpad-reviews.txt

jtvjml: you're looking at my db patch as well?  If so, tia :)07:16
jmljtv, np.07:17
thumperjml: ping07:59
jmlthumper, pong.07:59
thumperjml: re: removeSecurityProxy07:59
thumperjml: there is a big comment above as to why we need to change the datecreated07:59
thumperjml: are you explicitly mentining the use of the proxy in this case?08:00
jmlthumper, in this case, yes.08:00
thumperjml: the member isn't publicly accessable, hence the need08:00
thumperI'll add a comment08:00
jmlthumper, I'm remembering having this conversation about changing the datecreated, actually :)08:00
jmlthumper, why can't we make it accessible?08:00
* thumper thinks08:01
thumperno reason really I guess08:01
thumperit is readonly at the moment08:01
thumpernormally there is no need to modify the default value08:01
thumperthis is a special case08:01
thumperI'd rather not expose it for a special case08:01
thumperthat is all really08:01
jmlhmm.08:02
jmlBjornT, are you around?08:02
BjornThi jml 08:02
thumperBjornT: Karma.datecreated is currently readonly (like most datecreateds)08:03
jmlBjornT, ... and we want to backdate karma08:03
thumperBjornT: do we change it to read/write for the special case where we want to backdate karma08:03
thumperBjornT: or use removeSecurityProxy?08:03
jmlsee allocateKarma in lp.code.model.revision08:03
jmlhttps://code.edge.launchpad.net/~thumper/launchpad/revision-karma-fix/+merge/16825 for context08:04
thumperjml: well, it currently doesn't need the proxy removed, as it isn't wrapped at the point of access08:04
thumperbut by change to use the target to get it wraps it08:04
jml*nod*08:04
BjornTthumper, jml: without looking, it sounds like you should try to specify the right datecreated when creating the karma in the first place. would that be possible?08:05
jmlthumper, was just thinking that BjornT might want to look at the code to get some context as to _why_ we are fiddling w/ datecreated.08:05
jmlhmm. maybe.08:05
thumperBjornT: not through the person.allocateKarma method08:05
* jml looks.08:05
thumperwe could add yet another parameter I guess08:05
thumperassignKarma that is08:06
jmlthumper, yeah, that's what I was thinking.08:06
jmlthumper, that'd probably be nicer.08:06
jmlthumper, and wouldn't make the branch much bigger at all -- it's three lines of code08:06
thumperjml: exept for the tests to the change to assignKarma08:07
jmlthumper, I'm not going to make you test it :P08:07
jmlthumper, if you make it an optional parameter, you won't have to change any of the existing tests.08:08
thumperyeah...08:08
thumperalmost all params to assignKarma are optional08:08
jmll/reg/doc/person-karma.txt are tests, I think.08:09
thumperjml: slightly more than 3 lines: 5 files changed, 20 insertions(+), 12 deletions(-)08:17
jmlthumper, heh08:25
jmlthumper, well, I'm not in coding any more, I'm in product management -- everything ought to be simple, right?08:25
* jml recalls an account manager in a past job who insisted that every feature was only ever a weekend's work08:25
jmlI should try not to be that guy08:25
jmlanyway, people are putting drinks and delicious food in front of me -- good night.08:26
=== matsubara-afk is now known as matsubara
allenapgmb: When you have a mo, could you look at the post-review changes in https://code.edge.launchpad.net/~allenap/launchpad/checkwatches-stuff/+merge/16495?11:47
allenapbac: It's an old one, from before the hols, but when you have time can you have a look at the incremental changes I made to https://code.edge.launchpad.net/~allenap/launchpad/remove-xmlrpclib-marshalling-bug-254999/+merge/16436?11:48
gmballenap: Looking now.11:53
=== gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
gmballenap: You need to remove lib/lp/bugs/scripts/tests/test_checkwatches.py.moved11:57
allenapgmb: Gah, I hate computers.11:58
gmballenap: Other than that, r=me.11:59
allenapgmb: Thanks.11:59
=== gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: sinzui || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
allenapgmb: In rev 10054.7.1, there's a method NonConnectingBugzillaAPI.getProductsForRemoteBugs that refers to a bug_ids variable that doesn't exist. Things seem to be working anyway, so do you think it might be safe to drop that method?12:14
allenapgmb: Sorry, rev 10054.7.1 is where it was added.12:14
gmballenap: I have no idea... Drop it and see what happens. Is it in test_checkwatches.py?12:15
allenapgmb: Yep.12:15
gmballenap: Then it's part of a regression test... Let me take a look.12:15
allenapgmb: It runs without it.12:16
gmballenap: Then it should be safe to drop it.12:16
allenapgmb: Cool.12:16
=== gmb changed the topic of #launchpad-reviews to: on call: gmb[lunch] || reviewing: sinzui || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== jpds changed the topic of #launchpad-reviews to: on call: gmb[lunch] || reviewing: sinzui || queue [jpds(361650)] || 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: gmb[lunch], bac || reviewing: sinzui || queue [jpds(361650)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
bacgood morning13:31
bachi allenap, i'll have a look at your MP shortly13:33
=== bac changed the topic of #launchpad-reviews to: on call: gmb[lunch], bac || reviewing: sinzui, allenap || queue [jpds(361650)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
bacallenap: scratch that.  i see gmb already did.13:34
=== bac changed the topic of #launchpad-reviews to: on call: gmb[lunch], bac || reviewing: sinzui, jpds(361650) || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== mrevell is now known as mrevell-lunch
=== gmb changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: sinzui, jpds(361650) || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
gmbMorning bac.13:40
bachi gmb -- how's the frostbite?13:42
allenapbac: Hi Brad. Graham looked at a different mp, so my request still stands.13:42
allenapbac: If that's okay?13:42
bacallenap: ok, but i've already started on another.  will get to yours shortly.13:43
allenapbac: Brilliant, thanks.13:43
gmbbac: I'll survive. Perfectly pleasant 3-mile walk for milk this morning and all my extremities still in working order.13:43
=== bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: sinzui, allenap|| queue [jpds(361650) ] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
bacallenap: donning your former-build-engineer hat, do you know why 'ec2 land' would not submit to PQM after a successful test run?  at the end of the email is "successfully ran all tests" but no message about submitting to PQM.13:58
allenapbac: I don't know off the top of my head, but I'll take a look.13:58
=== gmb changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: mwhudson, allenap || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
allenapbac: ec2 land invokes ec2 test -s "message", so I can only think that the test runner failed after running tests, or didn't pick up a correct email configuration from your ~/.bazaar configuration.14:05
bacallenap: that would be odd.  it worked before the break and my config hasn't changed.14:06
bacallenap: i wonder if there is a trap for import_violations > METRIC_SHITLOAD14:07
allenapbac: Heh, yes, maybe :)14:07
sinzuiThe import violations are trivial fixes for the most part14:07
=== gmb changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, allenap || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
bacallenap: on your bug 254999 branch is the only difference from the last review that you fixed more files?  just more of the same?14:15
allenapbac: Yes, more of the same.14:16
bacallenap: looks good.  r=bac14:17
allenapbac: Thanks :)14:17
allenapbac: I can't find anything that causes the test suite to go boom if there are anti-fascist imports, but perhaps the fascist's atexit handler itself blew up... seems unlikely though.14:19
=== mrevell-lunch is now known as mrevell
bacallenap: ok, thanks for looking14:20
sinzuigmb: regarding the unclear error message in delete team: I wanted to it to be: I am sorry Dave, but I cannot let you craft an application to delete other people's data and gain control of their projects.14:34
sinzuigmb: I am not sure what to write given that the the message is to someone trying to break into Launchpad14:34
gmbHmm.14:35
gmbsinzui: Gimme a sec, just on a call; I'll have a think.14:35
=== matsubara is now known as matsubara-lunch
gmbsinzui: So, something simple like "unable to process submitted data"  - given this is an edge case and a nasty one at that we don't have to be terribly explicit. If you add a comment to explain why you're being vague that would work fine for me.15:07
sinzuiokay15:07
=== salgado is now known as salgado-lunch
=== bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
adiroibanjtv: hi. sorry for the delay. I was as bit bussy today15:26
jtvhi adiroiban!  I'm about to leave (in about -1.5 hours to be exact :)15:26
adiroibannp. tomorrow is also a day 15:26
adiroibani got stuck with modifying the 2 configure.zcml15:27
=== beuno is now known as beuno-lunch
=== jtv is now known as jtv-zzz
=== BjornT changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, - || queue [BjornT] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
BjornTgmb, bac: can anyone please review https://code.edge.launchpad.net/~bjornt/launchpad/enable-windmill-again?15:40
bacBjornT: i will15:41
BjornTthanks15:41
=== bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, BjornT || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
adiroibanabentley: hi. is there anything I need to do for https://code.edge.launchpad.net/~adiroiban/launchpad/bug-359180/+merge/16422 ?15:43
abentleyadiroiban: Has it passed the full test suite?15:44
adiroibanabentley: I only did the full test for translations module15:45
adiroibanmy computer is low on RAM and I can not do a full test15:46
adiroibanas I get an out of memory error15:46
adiroibanin soyuz 15:46
=== deryck is now known as deryck[lunch]
abentleyadiroiban: We can't land it until it passes the test suite.  Currently, I'm having trouble running the full suite myself.15:47
abentleyadiroiban: perhaps gmb or bac would be willing to run it through ec2 for you.15:47
gmbadiroiban, abentley: I'll take care of it.15:47
adiroibanabentley: ok. for the same reason I am stuck with https://code.edge.launchpad.net/~adiroiban/launchpad/bug-496352-series-status-refactor/+merge/1610815:47
gmbadiroiban: I'll do that, too :)15:47
abentleygmb: thanks.15:47
abentleygmb: x215:48
adiroibangmb: for  the series-status-refactor let me do a new merge 15:48
gmbWelcome ^215:48
adiroibanas it will probably fail after merging with devel 15:48
adiroibanthe branch is pretty old and there is a chance that someone has added a new DistroSeriesStatus 15:49
adiroibanthis will be merged without conflicts15:49
adiroibanbut since it was renamed to SeriesStatus15:49
adiroibanthe merged branch will be broken15:49
gmbadiroiban: Sure. Just ping me when ready.15:52
jpdsgmb: Can you also ec2 test https://code.edge.launchpad.net/~jpds/launchpad/fix_499997 for me?15:53
gmbjpds: Sure15:53
gmbjpds: Running15:55
jpdsgmb: Thanks.15:55
=== matsubara-lunch is now known as matsubara
=== bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -, - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
james_whi all, please to be reviewing https://code.edge.launchpad.net/~james-w/launchpad/sync-source-negative-versions/+merge/1685716:17
james_wgmb: ^ a bit of soyuz to warm your cockles?16:17
gmbEurgh.16:17
gmbjames_w: But since it's you.16:17
james_wheh16:18
=== gmb changed the topic of #launchpad-reviews to: gmb, bac || reviewing: james_w, - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
james_woh, did I get the target branch wrong?16:20
adiroibangmb: both branches should are ready for a full test. Thanks@16:21
gmbadiroiban: Okidoke. Will set them off shortly.16:21
adiroibangmb: no hurry16:22
gmbWTF: bzr: ERROR: xmlrpc protocol error connecting to https://xmlrpc.edge.launchpad.net/bazaar/: 302 Found16:23
gmbHmm.16:23
gmbadiroiban: Er, https://code.edge.launchpad.net/~adiroiban/launchpad/bug-496352-series-status-refactor/+merge/16108 appears to be 2000+ lines long and has 4 (apparently) unreviewed revisions in it. Can you take a look to see exactly what's gone on there please?16:30
adiroibangmb: it is a rename of DistroSeriesStatus to SeriesStatus16:32
gmbadiroiban: Ah, so it was always 2000+ lines?16:32
adiroibangmb: the new revisions are just fixing problems after merge16:32
adiroibangmb: yes16:32
gmbadiroiban: Okay, thanks.16:33
=== beuno-lunch is now known as beuno
gmbjames_w: Your change to security.py seems not relevant to the branch (I admit, I've not looked further down the diff yet). Is it?16:36
james_wgmb: it seems I started from the wrong branch/proposed for the wrong branch16:36
gmbOh.16:36
gmbThat's a new one.16:36
james_wI worked from devel, what should I propose merge in to?16:36
gmbOh, I see.16:36
gmbjames_w: launchpad/devel16:36
gmblp:launchpad is actually db-devel16:37
gmbFor stacking purposes.16:37
gmbjames_w: want me to reject that m-p?16:37
james_wI just deleted16:37
james_whttps://code.edge.launchpad.net/~james-w/launchpad/sync-source-negative-versions/+merge/1686116:39
* gmb looks16:41
gmbjames_w: There appear to be no tests for the change. I don't know Soyuz well enough to know whether there *can't* be any tests or not... can there?16:42
=== henninge_ is now known as henninge
gmb(I'm aware here that you might know the same amount as me on this front)16:42
james_w<james_w> where would I find the tests for ./scripts/ftpmaster-tools/sync-source.py ?16:43
james_w<bigjools> james_w: hahahaha16:43
james_wgmb: so I think no tests is the status quo16:44
gmbHah.16:44
gmbRight.16:44
gmbjames_w: In that case, r=me.16:44
james_wwould you be so kind?16:45
gmbjames_w: Sure.16:45
james_wI don't have PQM rights16:45
gmbFair dos.16:45
james_wthanks gmb 16:45
=== gmb 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
noodles775Hi bac - are you ok for a non-interactive review? (I'll be off in a few mins):16:52
noodles775https://code.edge.launchpad.net/~michael.nelson/launchpad/499095-build-retry-depwait-stuck/+merge/1686516:52
=== sinzui changed the topic of #launchpad-reviews to: on-call: bac || reviewing: - || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado-lunch is now known as salgado
bacnoodles775: sure16:53
noodles775Thanks.16:53
bacsinzui: why do you silently put things in the queue?  like pavlov's dog i only respond when i hear the bell.16:54
=== bac changed the topic of #launchpad-reviews to: on-call: bac || reviewing: - || queue [sinzui noodles] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
sinzuibac: because I have a cat dropping books on me16:54
sinzuiand it is now on meee and clawwinnng me16:54
bacgood reason, i guess16:55
baci could loan you jojo for a day and take care of that problem16:55
sinzuiI have seen jojo try to get in your lap. I do not think that is much help16:55
bacsinzui: thanks for showing a lplib test in your MP16:57
=== deryck[lunch] is now known as deryck
=== bac changed the topic of #launchpad-reviews to: on-call: bac || reviewing: sinzui || queue [noodles] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
sinzuibac: I have a whole script if you want16:57
bacsinzui: nah, i just usually have to nag people to test API changes with lplib16:58
sinzuibac: EdwinGrubbs pointed out that I had mis-spelled the parameters in the webservice test. That is what I have been trying to fix since 2:30 PM EST yesterday. I suck16:59
bacsinzui: spelling is not your strongest point16:59
sinzuiIt will be my demise.16:59
sinzuiabentley: rockstar: I want to resubmit a merge proposal to make it clear that it should be merged into db-devel, not devel. If I do so, will I loose all the excellent conversation and reviews that the proposal already has?17:16
rockstarsinzui, I don't think you can resubmit against another branch.17:16
* rockstar checks17:16
abentleysinzui: You can't resubmit with a different target branch.  And if you could, the excellent conversation would not be shown on the new proposal.  We are discussing changing both these things.17:17
sinzuiabentley: thanks for your insight. I do not think this is a common issue. jml/stuart should have caught the error when they started the review.17:18
=== sinzui changed the topic of #launchpad-reviews to: on-call: bac || reviewing: sinzui, jpds || queue [noodles] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
sinzuibac: to be clear, I am reviewing jpds's branch17:25
EdwinGrubbssalgado: can you mark this mp as approved? I fixed the last two issues you listed, but I hadn't notice that the status on your review was still needs-fixing. https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part2/+merge/1632517:51
salgadoEdwinGrubbs, sure17:52
salgadoEdwinGrubbs, what did you do with the code that was commented out?17:53
EdwinGrubbssalgado: I removed it entirely. It's just a message, and we normally show those with an info icon and no "Notice" header above it.17:55
bacsinzui: when i try to access findPerson(...) i get "Unrecognized parameters 'created_before' and 'created_after'"19:38
bacsinzui: this using your lplib snippet19:38
bacsinzui: what have i done wrong?  i rebuilt the wadl and removed my lplib cache file19:38
sinzuiinteresting19:39
sinzuiI had to 19:41
sinzuirm lib/canonical/launchpad/apidoc/wadl-development.xml; make lib/canonical/launchpad/apidoc/wadl-development.xml19:41
sinzuibac ^ which is what I think make build does when the wadl does not exist19:42
* bac tries19:44
* bac fails19:45
sinzuibac: is there any chance you are not using env=dev in your test script?20:02
sinzuiI can send you my test script20:02
bacsinzui: i'm definitely hitting dev.  i see the responses in the server window20:03
sinzuimake clean; make build ?20:04
bacdone20:04
sinzuido you see created_after in lib/canonical/launchpad/apidoc/wadl-development.xml ?20:04
bacsinzui: figured it out20:09
sinzuiplease tell20:10
bacthe cached wadl is *NOT* in api.launchpad.dev/cache but is in api.launchpad.dev/20:10
bacsinzui: so, now that i can play with it in lplib -- it works very well20:18
bacsinzui: r=bac20:24
sinzuithanks20:24
=== bac changed the topic of #launchpad-reviews to: on-call: bac || reviewing: noodles || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk
=== bac changed the topic of #launchpad-reviews to: on-call: bac || reviewing: jtv || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
rockstarmwhudson, can you do the review for this code-import-list branch?  I think it'll make you giggle.21:27
mwhudsonrockstar: sure21:27
mwhudsonrockstar: will i be embarrassed too?21:27
rockstarmwhudson, you shouldn't be.21:27
rockstarmwhudson, sending now.21:27
mwhudsoncool21:28
rockstarargh, forgot to sign me email.21:32
rockstarOne day, I'll be able to use my mind to verify my identity.21:32
mwhudsonrockstar: aaaaaaaaaaaargh!21:41
rockstarmwhudson, :)21:42
rockstarWhen I found it, I kind of chortled/facepalmed21:42
rockstarI was expecting it to be really complicated.21:42
rockstarthumper, could I get you to look at https://code.edge.launchpad.net/~rockstar/launchpad/branch-scanner-prep/+merge/16260 again?  I have some failing tests for the next branch, and then I can land these both.22:20
thumperrockstar: yes I'll look22:21
rockstarthumper, and as far as the <= is concerned, yes, I know it doesn't make sense.  It seems to be a storm oddity.  I had it the other way and couldn't get the test to pass, and then I noticed another garbo method had it that way, and changing it passed the test.22:28
=== bac 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

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!