/srv/irclogs.ubuntu.com/2010/03/25/#launchpad-reviews.txt

=== jelmer_ is now known as jelmer
=== flacoste is now known as flacoste_afk
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: wgrant || queue: [jelmer,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775Hi wgrant, in addition to the lucid-related failure with your soyuz-upload.txt, I also see a bunch of deprecation warnings after line 746 (printing stderror).08:32
* wgrant looks.08:33
intellectronicanoodles775: hi, i have a tiny js/ui review for you (that placement fix jtv asked for). can i add it to the queue?08:35
wgrantnoodles775: That *is* odd.08:36
noodles775intellectronica: of course.08:37
wgrantWhy did that not appear before...08:37
noodles775wgrant: so it wasn't there ear...08:37
=== intellectronica changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: wgrant || queue: [jelmer,wgrant, intellectronica] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775yeah.08:37
wgrantAh.08:38
wgrantGuess which package was upgraded overnight.08:38
noodles775aha.08:38
wgrantI guess I could filter those warnings in ftpmaster.py08:39
wgrantBut it probably makes other stuff fail too.08:39
wgrantIt's tempting to leave that for now, rerun the test suite tonight, and fix all the introduced failures in one lot tomorrow.08:39
noodles775wgrant: yeah, since I assume there will be a whole bunch of new lucid errors for soyuz tests :/08:40
wgrantI think filtering those warnings in a couple of places should do it, but we'll find out.08:40
wgrantThat test should pass fine on Hardy still.08:40
noodles775OK. I'll update the MP.08:40
wgrantYay for huge API changes in the last month of an LTS.08:41
noodles775wgrant: sorry, just to check: do you want me to approve and land this branch, since the tests will pass on hardy?08:42
wgrantThanks.08:42
wgrantnoodles775: Please do.08:42
wgrantI'll fix all the new Lucid failures tomorrow.08:43
noodles775Wonderful :)08:43
noodles775wgrant: with your rescueiflost/updatestatus branch, couldn't we do something like rescueIfLost = Builder.rescueIfLost on the MockBuilder if the code is the same? (and not have to define the functions?)09:10
wgrantnoodles775: Bound methods check the type of the first argument.09:10
wgrantSo no, it's not quite that easy :(09:10
wgrantEr, unbound methods.09:10
noodles775Ok.09:10
noodles775wgrant: in which case, is there an obvious reason that I'm missing for why MockBuilder couldn't inherit from Builder and just overload all its methods as needed?09:22
wgrantnoodles775: It looks like a few other mocks do that. So maybe I could.09:27
noodles775wgrant: OK, I'll leave it up to you - I personally think it would improve the code.09:27
noodles775r=me09:27
wgrantnoodles775: It would, yes.09:27
wgrantThanks.09:27
* wgrant will fix.09:27
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: intellectronica || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775intellectronica: where's your MP? I can't see it on active reviews?09:33
intellectronicanoodles775: because it's a lazr branch. https://code.edge.launchpad.net/~intellectronica/lazr-js/choiceedit-horizontal-position/+merge/2210609:34
noodles775Thanks.09:34
wgrantnoodles775: Mmmm, because Builder inherits SQLBase it's not that simple. Simply initialising it tries to add it to the DB, and then all hell breaks loose...09:48
noodles775wgrant: :/09:50
wgrantSo it's probably best to leave it as it is now, although it's ugly.09:56
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: intellectronica || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775intellectronica: I approved your MP, but please see the subsequent comment and see what you think.10:18
noodles775I didn't think it was an issue at first (and hence didn't discuss it here), but it may be worth considering.10:19
intellectronicanoodles775: yeah, i'm not sure what's the appropriate solution (and it's very much a corner case). we could avoid doing the correction, so that at least you get to see the leftmost content10:21
intellectronicathen again, is it really likely that people will be using this from a screen smaller than the widget?10:21
intellectronicaon my phone, for example, there's enough space for the widget to display10:22
noodles775intellectronica: yeah, it is a corner case, and I don't know what the average pixel width of a phone is. Given that it's not hard to avoid it, I personally think it'd be worth the extra line or two, but up to you.10:25
noodles775intellectronica: that is, if you avoid the correction in that case, the user can always scroll right to see the rest. But in the current case, they can never scroll left beyond 0.10:25
intellectronicanoodles775: yeah, i think simply not doing the correction is the right way to go. the widget is going to be cut anyway, but at least it will be cut on the right.10:25
=== wgrant changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: intellectronica || queue: [StevenK,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: - || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775jelmer_: do you want to look at wgrant's first?10:51
jelmer_noodles775: sure11:02
wgrantjelmer's a mentat now?11:06
noodles775Yeah.11:10
wgrantNice.11:11
noodles775[, 1011:24
noodles775oops, wrong kb layout ;)11:24
bigjoolsare you a crazy dvorak dude?11:25
=== jelmer_ is now known as jelmer
* wgrant would recognise that mistake anywhere.11:26
=== noodles785 is now known as noodles775
=== matsubara-afk is now known as matsubara
=== jelmer changed the topic of #launchpad-reviews to: on call: noodles775, jelmer || reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado_ is now known as salgado
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775, jelmer || reviewing: wgrant,sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775, jelmer || reviewing: wgrant, lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
leonardrnoodles775, jelmer, i need review of two tiny branches12:57
leonardrhttps://code.edge.launchpad.net/~leonardr/launchpad/lucid-updates12:57
leonardrhttps://code.edge.launchpad.net/~leonardr/launchpadlib/use-1.012:57
leonardrnoodles775, jelmer: actually just the lucid-updates one12:59
leonardri'm going to redo that merge proposal actually to get rid of conflicts12:59
leonardrnoodles775: https://code.edge.launchpad.net/~leonardr/launchpad/lucid-updates/+merge/2212313:04
=== flacoste_afk is now known as flacoste
=== wgrant changed the topic of #launchpad-reviews to: on call: noodles775, jelmer || reviewing: wgrant, lunch || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
wgranthttps://code.edge.launchpad.net/~wgrant/launchpad/sprb-package-branch-db-permission/+merge/22128 <- about as trivial as they get.13:32
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775, jelmer || reviewing: wgrant, wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775lol13:49
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775, jelmer || reviewing: bigjools, wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara is now known as matsubara-lunch
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775, jelmer || reviewing: -, wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
leonardrnoodles775, still waiting for a review of https://code.edge.launchpad.net/~leonardr/launchpad/lucid-updates/+merge/2212314:26
noodles775leonardr: I approved it 40mins ago?14:27
noodles775s/40/31 ;)14:27
leonardr??14:28
leonardrnoodles775: so you did, for some reason it still says needs review14:28
noodles775leonardr: yes, the status doesn't update automatically, as you might need multiple reviews (ui/code etc.) before you want to set the MP as approved.14:29
leonardrok, i guess i don't do many launchpad branches14:30
noodles775leonardr: you've got the perms to update the status though right?14:31
leonardrnoodles775: yes14:31
noodles775Great.14:31
=== wgrant changed the topic of #launchpad-reviews to: on call: noodles775, jelmer || reviewing: -, wgrant || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775, jelmer || reviewing: wgrant, wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775wgrant: around? With your MP, just wondering if you think it's worth adding ISPRecipe.base_branch (simply proxy to SPRData.base_branch which already exists) for use in the title?15:13
noodles775Instead of including *all* the referenced branches?15:13
wgrantnoodles775: I considered that. I guess including all could get a bit long sometimes.15:13
noodles775Yeah.15:13
noodles775With that, r=me.15:14
noodles775oh, I didn't send of your second branch to land yet...15:14
=== noodles775 changed the topic of #launchpad-reviews to: on call: jelmer || reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* noodles775 retires for the day to do some qa.15:16
wgrantThanks noodles775.15:16
=== matsubara-lunch is now known as matsubara
wgrantnoodles775: I've fixed that branch. Do you have a moment to send it off to EC2?15:37
noodles775wgrant: Sure.15:37
wgrantlp:~wgrant/launchpad/sprb-package-branch-db-permission appears to be approved too -- is that in EC2 somewhere? I guess it can probably just land directly.15:39
=== salgado is now known as salgado-lunch
=== jelmer changed the topic of #launchpad-reviews to: on call:|| reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== deryck is now known as deryck[lunch]
=== salgado-lunch is now known as salgado
=== deryck[lunch] is now known as deryck
=== salgado 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
=== salgado is now known as salgado-afk
=== fjlacoste is now known as flacoste

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