/srv/irclogs.ubuntu.com/2010/02/26/#launchpad-reviews.txt

=== sinzui changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: leonardr || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
sinzuirockstar: if you are still reviewing: https://code.launchpad.net/~sinzui/launchpad/something-and-nothing/+merge/2019100:20
=== rockstar 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
rockstarsinzui, looking01:26
=== mup_ is now known as mup
=== jelmer changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [al-maisan/jelmer] || This channel is logged: http://irclogs.ubuntu.com ||
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== jtv is now known as jtv-afk
al-maisanGood morning adeuring, could you please take a look at https://code.edge.launchpad.net/~al-maisan/launchpad/restricted-ui/+merge/20145 ?11:07
adeuringhi al-maisansure, I'll look11:07
al-maisanthe diff is not quite right, this is all the branch does: http://pastebin.ubuntu.com/384297/11:07
al-maisanadeuring: ^^11:08
adeuringsl-thanks!11:08
=== bigjools changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
bigjoolsadeuring: simple branch for ya11:12
adeuringbigjools: sure; i just started a review for al-maisan, after that one?11:12
bigjoolsnp11:12
al-maisando we need a database review for a branch that modifies database/schema/security.cfg ?11:15
adeuringal-maisan: r=me; 1 nitpick11:23
al-maisanadeuring: thanks :)11:23
al-maisanadeuring: I'll take care of the nitpick11:24
adeuringbigjools: r=me11:34
bigjoolsadeuring: thanks very much11:35
bigjoolsno nitpicks? :)11:35
noodles775Hi adeuring, I've got an 820 line pure test refactoring branch needing review, but if you'd prefer, I can move the last change out and into the next pipe: https://code.edge.launchpad.net/~michael.nelson/launchpad/ppa-privatisation-test-refactor5/+merge/2020711:38
jelmerhi adeuring11:38
jelmeradeuring: When you have a chance, can you have a look at https://code.edge.launchpad.net/~jelmer/launchpad/bug471148-ui/+merge/20208 ?11:38
jelmerHmm, today seems to be soyuz review day :-)11:38
bigjoolswe're productive dudes11:39
adeuringnoodles775: na... that's fine11:39
=== noodles775 changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [bigjools, noodles775, jelmer] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
adeuringjelmer: sure, i can look.11:39
noodles775Thanks adeuring !11:39
jelmeradeuring: Thanks11:41
gmbadeuring: Okay if I add another branch to the queue?11:41
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: noodles775, || queue [jelmer] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== gmb changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [jelmer, gmb] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== gmb changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: noodles775 || queue [jelmer, gmb] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
adeuringgmb: sure, but I just strted a somewhat longer review for noodles77511:42
gmbadeuring: Okay, I'll see if I can find someone else to reviw my branch.11:42
salgadogmb, I'll start my shift shortly and I'll have plenty of time to review yours12:46
=== matsubara-afk is now known as matsubara
gmbsalgado: allenap's kindly agreed to review it in the meantime; sorry, I forgot to remove it from the queue. Thanks anyway.13:02
=== gmb changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: noodles775 || queue [jelmer] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
salgadono worries13:05
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: jelmer || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775Thanks adeuring .13:28
=== bigjools changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: jelmer || queue [bigjools] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
bigjoolsadeuring: I have a further tweak to the change you just reviewed, it's on the queue.13:38
adeuringbigjools: ok13:38
bigjoolsI have to run out to give a presentation on FLOSS to my son's school now!13:38
jtv-afkbigjools: run, julian, run!13:44
=== bigjools is now known as bigjools-lunch
adeuringjelmer: could you add doc strings to _get_arm_builds_enabled() and _get_arm_builds_disabled()?13:46
adeuringjelmer: sorry... i mean _set_arm_builds_enabled() instead of the "disabled"13:47
jelmeradeuring: ok13:47
adeuringjelmer: there is an "elif" in set_arms_builds_enabled() without a final "else". Could you add such an "else: pass", together with a comment why nothing needs to be done in this case?13:49
adeuringjelmer: and (sorry about all these nitpick for one method...): The comment "a link is required but it is present" is a bit enigmatic, at least for me. What about sonething like "Arms builds are already enabled"?13:51
jelmeradeuring: Sure, that's more sensible indeed.13:51
adeuringjelmer: getPubSource() in test_publishing.py needs a doc string13:56
adeuringjelmer: other than these nitpicks, r=me14:00
jelmeradeuring: Thanks!14:00
adeuringjelmer: welcome14:01
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: bigjools || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== fjlacoste is now known as flacoste
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== al-maisan_ is now known as al-maisan
=== bigjools-lunch is now known as bigjools
noodles775Hi again adeuring ! I've got the final test refactoring branch ready if you've time: https://code.edge.launchpad.net/~michael.nelson/launchpad/ppa-privatisation-test-refactor6/+merge/2022714:59
adeuringnoodles775: could you ask salgado? I have a somewhat urgent branch myself...14:59
noodles775Sure, salgado?14:59
=== salgado changed the topic of #launchpad-reviews to: on call: adeuring,salgado || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
salgadonoodles775, I'm working on a critical issue, but if gary_poster can't take it I will15:01
gary_posternoodles775, salgado: looking15:02
=== gary_poster changed the topic of #launchpad-reviews to: on call: adeuring,gary_poster || reviewing: ?,noodles775 || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
gary_postersalgado: changed topic taking you off reviewers15:06
=== jtv-afk is now known as jtv
salgadothanks gary_poster15:06
gary_posternoodles775: why did you remove test_login.py?15:06
noodles775gary_poster: I didn't... a local diff between this pipe and the prev shows everything but the test_login.py.15:07
noodles775I was just trying to figure out what was going on with the MP.15:07
gary_posternoodles775: ok, I'll just clarify in the review that merging that removal is not ok, and leave you to handle the mechanism :-)15:08
noodles775gary_poster: I'll merge RF into the initial pipe (there are 7 in total) and push it through and see if it's still there when the diff regenerates on the MP.15:11
gary_posternoodles775: ack, ok15:12
salgadogary_poster, noodles775, in my openid branch I added a test_new_login.py file and later removed test_login.py.  then on a subsequent branch I renamed test_new_login.py to test_login.py.  that, together with pipes, might have confused  bzr?15:14
noodles775salgado: thanks for the info - I thought it may have been related to your branch I reviewed yesterday. If that's the case, pumping a fresh RF through my pipes should resolve the issue. (I hope)15:15
gary_posternoodles775: in xx-private-builds.txt, why is it OK to remove diff line 375ff? ("Now visit the same page as an admin we can see that there are four more...")15:15
gary_posterthanks salgado15:15
gary_posternoodles775: also, in the previous section of the same file, why did the result change (5 of 17 results, and different values)?  If explaining it takes too long, I'll accept "because of changes that have previously passed review"15:17
gary_posterbut I'd like reassurance that you are confident of the change, at least :-)15:18
gary_posterhm, I could understand an increase of 1, since we added the private p3a archive, but I don't understand 14 -> 1715:19
=== jtv is now known as jtv-afk
noodles775sorry gary_poster, I'll be there in a minute or two.15:24
gary_posternp noodles77515:24
stubgary_poster: You already have looked at https://code.edge.launchpad.net/~stub/launchpad/memcache/+merge/20226 earlier. Its finished now.15:30
noodles775gary_poster: so, regarding your first question, 375ff, the test for authorised viewers seeing the builds in the history was moved to just below with the Frog builder.15:31
noodles775It looks like a lot less test-code because the Frog builder only has the one build in its history (it's not polluted with sample data :)).15:31
stub(it won't land this cycle though if you need to prioritize)15:31
gary_posternoodles775: ah ok cool thanks15:32
noodles775gary_poster: and your second question is related, because previously cprov's ppa was switched to private (which is no longer allowed), and had a bunch of publishings.15:32
gary_posterstub: cool, I'll take it, unless flacoste, who was just complimenting the branch, tells me that he'll take it.  But I'll shepherd it one way or the other15:33
noodles775So the 14 -> 17 is because there are 3 packages in cprov's public archive (when it was private ,they did not show up).15:33
gary_posterstub: yeah, I'll take it :-)15:33
flacostestub: it's not a review, but heh, nice work! i like the syntax a lot!15:33
stubta :)15:33
=== sinzui changed the topic of #launchpad-reviews to: on call: adeuring,gary_poster || reviewing: ?,noodles775, sinzui || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775s/packages/builds of course.15:34
gary_posternoodles775: we dupe the private setup in xx-source-package-publishing.txt and xx-binary-package-publishing.txt (">>> from lp.registry.interfaces.distribution import IDistributionSet...").  You considered and rejected a helper function?  Two copies is arguable, maybe.  Three, much less so. :-)15:38
gary_posterSo if you considered and rejected then I'm OK, but otherwise, please consider :-)15:39
sinzuibac: EdwinGrubbs: are either of you available to review https://code.launchpad.net/~sinzui/launchpad/timeout-portlet-package-summary-2/+merge/2022815:41
noodles775gary_poster: no I didn't consider it - sounds good.15:41
gary_posterok cool noodles77515:41
bacsinzui: i can do it later.  i just made a break through on my testing so i'd prefer not to interrupt that effort ATM15:42
sinzuirock15:42
allenapadeuring, gary_poster: Are either of you close to be able to review another branch? If not I'll see if I can get a quick review elsewhere.15:43
gary_posterallenap: I'd be able to do it in about...30-45 min, I'd guess15:44
allenapgary_poster: Okay, I'll go and prod someone else. (I don't have much time left to work today, that's the only reason.) Thank you though :)15:46
gary_posterallenap: cool, np :-)15:46
gary_posternoodles775: I'm finishing the review, but merge-conditional on not landing the deletion of test_login.py15:46
adeuringallenap: I can review it in a few miuntes15:47
gary_posterI mean, I'm writing it up15:47
gary_posterbut it is r=gary with that15:47
allenapadeuring: deryck is doing it now, but thank you anyway.15:47
noodles775gary_poster: thanks. Actually, if you re-load the MP you'll see that it's no longer there (pumping a fresh RF through seems to have helped)15:47
gary_posternoodles775: confirmed, awesome :-)15:48
=== matsubara is now known as matsubara-lunch
=== salgado is now known as salgado-lunch
=== jtv-afk is now known as jtv
noodles775Hi gary_poster, would you object if I land that earlier branch without creating the pagetest helper? I need to leave 20mins ago and would like to send it off to ec2.16:49
gary_posternoodles775: that's fine16:51
noodles775gary_poster: ta.16:51
=== matsubara-lunch is now known as matsubara
=== salgado-lunch is now known as salgado
=== deryck is now known as deryck[lunch]
=== danilo__ is now known as daniloff
=== deryck[lunch] is now known as deryck
=== sinzui changed the topic of #launchpad-reviews to: on call: adeuring,gary_poster || reviewing: ?,noodles775 || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== adeuring changed the topic of #launchpad-reviews to: on call: gary_poster || reviewing: noodles775 || queue [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
sinzuibac: EdwinGrubbs: can either of you review my branch: https://code.launchpad.net/~sinzui/launchpad/timeout-portlet-package-summary-2/+merge/20228 If we cannot land it, we need to RC and remove code next week18:24
bacsinzui: yes18:24
bacsinzui: is there a bug to put the 'needs linking' section back in?18:35
bacassuming we can solve the query timeouts18:35
sinzuino there is not18:39
sinzuibac: I assume we need a model change and job to do daily updates18:39
bacsinzui: am i right in assuming we want to restore it?  i just don't want to rip it out and then forget about it.18:40
sinzuibac: yes we should restore it after the next two blueprints18:41
sinzuiI can report a bug and add an XXX18:41
baccool18:41
bacdone18:41
sinzuibac: I was able to restore the template. I changed view/needs_linking to return None and left an XXX. We can re-enable the template by uncommenting a line19:21
bacah, sweet19:22
=== jtv is now known as jtv-eat
=== gary_poster is now known as gary-lunch
intellectronicacan anyone take a small branch for review?20:55
intellectronicai see that there's a bit of a queue, and it's getting late for me, so if anyone other than gary wants to snatch it now i'll be grateful20:56
intellectronicaonly 115 lines20:56
intellectronicarockstar: how about you? you're being suspiciously quiet20:57
intellectronicaEdwinGrubbs: maybe you fancy a short review?20:58
EdwinGrubbsintellectronica: I should able to do a review in about 30 minutes.20:59
intellectronicaEdwinGrubbs: wow, you're amazing. thanks so much. i'm creating an MP now20:59
intellectronicaEdwinGrubbs: https://code.edge.launchpad.net/~intellectronica/launchpad/heat-decay-complete/+merge/2025021:04
gary-lunchsinzui: do you have a branch you need me to look at?21:14
=== gary-lunch is now known as gary_poster
sinzuigary, no, sorry, bac reviewed it for me21:14
=== sinzui changed the topic of #launchpad-reviews to: on call: gary_poster || reviewing: noodles775 || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
gary_postercool21:14
=== gary_poster changed the topic of #launchpad-reviews to: on call: gary_poster || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
bacgary_poster: you ditched us last night!21:15
gary_posterbac: :-P sorry.  talked later with chris rossi at the office and then decided I had had my "drink".  I'll stay later next time...esp. when it is closer to home. :-)21:16
gary_posterbac, do you thnk you will do any of the hack nights?21:16
bacyes, probably.  especially if i make any progress on my side project, which hasn't gotten off the ground yet21:17
=== jtv-eat is now known as jtv
EdwinGrubbsintellectronica: review sent21:45
intellectronicaEdwinGrubbs: great, thanks a million!21:46
bdmurrayintellectronica: so I've figured out what was wrong with export bug linked branches branch21:49
intellectronicabdmurray: oh yeah? what was it?21:49
bdmurrayintellectronica: I believe we were importing and using the wrong thing.  But now I'm uncertain how to proceed with a new or modified merge proposal21:50
intellectronicabdmurray: excellent. i'm just about to sign off, but i'll be back tomorrow, so if you have something ready i can review and land it for you21:51
bdmurrayintellectronica: okay, cool.  Should I just make a whole new merge proposal against devel and reference the old one?21:51
intellectronicabdmurray: sure, that works21:51
=== matsubara is now known as matsubara-afk
=== gary_poster 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
=== jamalta is now known as jamalta-afk

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