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

=== jamalta-afk is now known as jamalta
=== jamalta is now known as jamalta-afk
=== jtv is now known as jtv-otp
=== noodles785 is now known as noodles775
=== jtv-otp is now known as jtv
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
al-maisannoodles775: consider yourself hassled! https://code.edge.launchpad.net/~al-maisan/launchpad/unembargo-oops-526645/+merge/2112609:44
noodles775ha!09:44
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: - || queue: [al-maisan] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: -, - || queue: [al-maisan] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
jtvnoodles775, al-maisan: I've got 3 small ones for you... interested?09:55
jtvtiny: https://code.edge.launchpad.net/~jtv/launchpad/bug-535661/+merge/2104509:55
al-maisanjtv: otp, should be available in a few minutes..09:55
* jtv is patient09:55
al-maisanjtv: that's very much appreciated.09:56
jtv:)09:56
noodles775jtv: sure! Just pop them in the queue.09:57
jtvpop or push, that is the question...09:57
jtvon call: noodles775, al-maisan || reviewing: -, - || queue: [al-maisan, jtv3] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews09:57
al-maisannoodles775: I can start on jtv's stuff ..10:05
* jtv nods frantically :)10:05
noodles775al-maisan: yep, I'll be starting on yours in a tick, just updating some bugs.10:07
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: -, jtv || queue: [al-maisan] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775al-maisan: there were three for jtv right?10:15
jtvright10:15
noodles775jelmer was going to grab one too.10:15
al-maisangreat10:15
* al-maisan is looking at https://code.edge.launchpad.net/~jtv/launchpad/bug-535661/+merge/2104510:15
jelmerI'll take https://code.edge.launchpad.net/~jtv/launchpad/bug-536769/+merge/21068 in that case10:15
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: al-maisan, jtv || queue: [jtv*2] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
jtvjelmer: dankjewel10:16
al-maisanhmm .. the one I picked is a hard nut ;)10:16
* al-maisan moves on to https://code.edge.launchpad.net/~jtv/launchpad/cleanups-475435/+merge/2107010:21
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: al-maisan, jtv2 || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775al-maisan, jtv: do you guys know where the old mentat guidelines are (things to watch out for as a new reviewer).10:21
* al-maisan digs around for the guidelines10:21
al-maisanhttps://dev.launchpad.net/PreMergeReviews10:22
jtvnoodles775: the old ones were on the old internal LP wiki IIRC10:22
noodles775jtv: yeah, that's where I'm searching.10:22
noodles775(without success)10:22
jtvbut someone or other went to some effort to reorganize a lot of that stuff on the new one10:22
al-maisanhttps://dev.launchpad.net/PythonStyleGuide10:23
noodles775al-maisan: yeah, the styleguide and review preparation notes are on the new wiki, but there used to be a page specifically for new reviewers (with lots of suggestions etc.)10:23
al-maisanhttps://dev.launchpad.net/WorkingWithReviews10:23
jtvhttps://launchpad.canonical.com/MentorProcess10:24
jtvhttps://launchpad.canonical.com/TipsForReviewers10:24
noodles775jtv: gold! THanks!10:24
jelmerjtv: thanks10:27
al-maisanjtv: looking at lp:~jtv/launchpad/cleanups-475435, line 23 of the diff: why the question mark at the end of the line?10:55
jtval-maisan: I was just looking for a way to express the meaning of the parameter... if you have a better idea, I'd be happy to change it.10:56
al-maisanah10:57
al-maisanjtv: your cover letter does not specify any tests to run .. how did you test the change locally?10:57
jtval-maisan: tbh I don't remember...  I know I didn't run the windmill tests, but other than that, really don't know now10:58
jtvI have a vague recollection of running the pagetests only10:59
* jtv tries running those now10:59
al-maisanjtv: please paste the command here ..11:00
jtval-maisan: ./bin/test -vv -t 'translations.*stories'11:00
al-maisanthanks!11:00
jtval-maisan: I think I broke something, though it may also be the buildfarm stuff I'm running on the side.  Want me to retract this one, have EC2 run a full test, and try again later?11:03
al-maisanjtv: FWIW I am seeing test failures on my side as well11:04
al-maisanwith fresh trunk + your branch merged11:04
* al-maisan waits for the test run to finish11:05
jtvmaybe I just missed the test results between reboots or something...  I'll retract this one.11:05
al-maisanjtv: fine.11:05
al-maisanjtv:11:07
al-maisan  Ran 863 tests with 7 failures and 0 errors in 4 minutes 34.474 seconds.11:07
al-maisanTests with failures:11:07
al-maisan   lib/lp/translations/tests/../stories/standalone/xx-person-activity.txt11:07
al-maisan   lib/lp/translations/tests/../stories/standalone/xx-pofile-details.txt11:07
al-maisan   lib/lp/translations/tests/../stories/standalone/xx-series-templates.txt11:07
al-maisan   lib/lp/translations/tests/../stories/standalone/xx-translationmessage-translate.txt11:07
al-maisan   lib/lp/translations/tests/../stories/standalone/xx-translations-to-complete.txt11:07
al-maisan   lib/lp/translations/tests/../stories/standalone/xx-translations-to-review.txt11:07
al-maisan   lib/lp/translations/tests/../stories/importqueue/xx-translation-import-queue.txt11:07
jtval-maisan: I think the easiest is just to discard the branch...  it was really just leftovers from exploring a different problem.11:07
jtvSorry for putting you to the trouble.11:08
al-maisanfair enough .. I'll stop looking at it then.11:08
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: al-maisan, jtv2 || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
* al-maisan moves on to https://code.edge.launchpad.net/~jtv/launchpad/bug-536769/+merge/2106811:09
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: al-maisan, jtv3 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775al-maisan: I'm not sure if jelmer already started that one before he had to pop out?11:10
jtvnoodles775: I think he did, yes11:10
al-maisannoodles775: it will need another review anyway11:10
al-maisansince jelmer has not graduated as a reviewer yet11:10
al-maisanright?11:10
* al-maisan can hold off11:11
noodles775al-maisan: True, it will, but that should happen *after* his.... yeah, take an ocr break ;)11:11
al-maisanssure11:11
noodles775al-maisan: btw, I'm seeing if your queries can be done simply with storm.11:11
al-maisannoodles775: IMHO there is no advantage in doing so since the queries are not fetching full objects that can be cached11:13
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: al-maisan, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775al-maisan: you can use result_set.values() to just get specific columns, but it's not for an advantage that I'm looking at it, but because (whether I like it or not) it's the ORM we're trying to use.11:15
noodles775Assuming there is no disadvantage for the query of course.11:15
noodles775al-maisan: sheesh, the required imports for doing the storm version are a pain in and of themselves! ;)11:34
al-maisanhmm..11:47
stubhttps://code.edge.launchpad.net/~stub/launchpad/memcache/+merge/21130 (trivial)11:47
noodles775stub: r=me, I was wondering why we're not using the title attribute for the abbr tags though?11:57
=== matsubara-afk is now known as matsubara
=== jelmer changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: al-maisan, - || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
stubnoodles775: No idea on title. Ta for the review though :)12:19
stubnoodles775: Should I add the titles to the abbreviations since I'm here?12:19
noodles775stub: yeah, that'd be great... Thanks.12:20
stubWhat is the title for CVS?12:21
noodles775Concurrent Versioning System?12:21
* noodles775 checks12:21
stubConcurrent Versions System12:22
stubOh - we don't use title cause it works crap in the UI, double mouse overs because we are in an <a> tag.12:24
stubI can spell SVN Subversion easily enough12:25
noodles775urg, there you go.12:25
noodles775What's the mouse over for the <a> though? There's no title on it? Weird.12:26
stubWe underline on hover and the mouse pointer changes12:28
stubBut if you mouse over the abbr you get the ? icon and not the finger icon12:28
noodles775I see, yeah, that sux.12:29
stubI'll land it as is since I've already run the tests ;)12:30
noodles775Great.12:30
adiroibandanilos: hi. do you have time for talking about bug 201749?12:48
mupBug #201749: Impossible to mark as needing review strings already translated or with suggestions <story-translations-review> <ui> <Launchpad Translations:In Progress by adiroiban> <https://launchpad.net/bugs/201749>12:48
danilosadiroiban, some, sure12:49
adiroibanbrowser/translationmessage.py 419-42012:50
adiroibandanilos: from here http://bazaar.launchpad.net/~adiroiban/launchpad/bug-201749/revision/1046712:50
adiroibanI think they are needes, as if user is not official translator12:50
adiroibanthose values will not be set to false12:50
danilosadiroiban, how come?12:51
danilosadiroiban, they won't be present in the form, thus .get() will use the default value of False12:51
adiroibandanilos: but maybe someone will forge the form12:51
adiroibanand inject those values12:52
danilosadiroiban, so?12:52
adiroibanand so force_diverge can be set to True12:53
adiroibaneven if the user is not a reviewer12:53
danilosadiroiban, privileges are still checked in updateTranslation if I am not mistaken12:53
danilosadiroiban, so, it doesn't really matter12:54
adiroibandanilos: ok. my bad. But I didn't find the place in updateTranslation12:55
danilosadiroiban, (I may have mentioned how updateTranslation is a nasty behemoth; well, this is one part of it :)12:55
danilosadiroiban, look for force_diverged and where just_a_suggestion is set; updateTranslation *is* complex12:56
adiroibanforce_diverges is used in _makeTranslationMessageCurrent12:56
adiroibanand I can not see the security check there12:57
adiroibanhttp://paste.ubuntu.com/393247/12:57
adiroibandanilos: but we can talk about this later and I will use staging to test if this is a security problem12:58
danilosadiroiban, that path is not entered unless just_a_suggestion is False12:58
danilosadiroiban, you can spend time checking it, but I can assure you it's not ;)13:00
adiroibandanilos: ok :)13:00
danilosadiroiban, if you want, read the code to confirm it (_isTranslationMessageASuggestion is used to set just_a_suggestion)13:00
adiroibandanilos: thanks :)13:01
=== henninge_ is now known as henninge
adiroibandanilos: I still don't know how diverged messages should be handled in this case http://paste.ubuntu.com/393253/13:05
jelmernoodles775, al-maisan: can either of you have a look at my branches?13:06
adiroibanI don't understand why the new translations are going to be diverged by default13:07
danilosadiroiban, not always, only if the existing one is already diverged13:07
adiroibandanilos: true. but why, or how are they going to be diverged?13:07
noodles775jelmer: after lunch I can sure.13:08
danilosadiroiban, we've got thousands of those already, we can't just close our eyes on it13:08
=== mrevell is now known as mrevell-lunch
=== salgado is now known as salgado-afk
adiroibandanilos: is this a valid usecase for this bug https://bugs.edge.launchpad.net/rosetta/+bug/201749/comments/14 ?13:15
mupBug #201749: Impossible to mark as needing review strings already translated or with suggestions <story-translations-review> <ui> <Launchpad Translations:In Progress by adiroiban> <https://launchpad.net/bugs/201749>13:15
danilosadiroiban, sorry, on the phone13:16
=== mrevell-lunch is now known as mrevell
=== jamalta-afk is now known as jamalta
=== al-maisan changed the topic of #launchpad-reviews to: on call: noodles775, al-maisan || reviewing: al-maisan, jelmer || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
al-maisannoodles775, jelmer: I am looking at https://code.edge.launchpad.net/~jelmer/launchpad/bug531985/+merge/2103814:05
noodles775al-maisan: great, I'm off to lunch :)14:05
al-maisannoodles775: did you rewrite my branch yet? ;)14:07
al-maisannoodles775: ah, just saw the review email, thanks!14:09
al-maisanjelmer: how was lp:~jelmer/launchpad/bug531985 tested?14:25
=== matsubara is now known as matsubara-lunch
=== noodles775 changed the topic of #launchpad-reviews to: on call: al-maisan || reviewing: jelmer || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara-lunch is now known as matsubara
=== rockstar changed the topic of #launchpad-reviews to: on call: al-maisan, rockstar || reviewing: jelmer, || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
al-maisan[15:25] <al-maisan> jelmer: how was lp:~jelmer/launchpad/bug531985 tested?16:21
=== deryck is now known as deryck[lunch]
jelmeral-maisan: sorry16:29
jelmeral-maisan: I tested by adding myself to commercial-admins16:29
al-maisanjelmer: on your local system?16:30
al-maisans/local/development/16:30
al-maisanjelmer: r=me16:37
jelmeral-maisan: dogfood only16:37
jelmeral-maisan: sorry, too much conversations happening at once :_/16:37
al-maisanjelmer: that's even better :-)16:37
jelmeral-maisan: thanks16:37
=== deryck[lunch] is now known as deryck
=== gary_poster is now known as gary-lunch
* kfogel is away: switching consoles for a bit, back soon18:19
leonardrgary, this isn't quite ready for review, but could i get your feedback on https://code.edge.launchpad.net/~leonardr/launchpad/multiversion-apidoc ?18:23
leonardrthe incremental diff is http://pastebin.ubuntu.com/393449/18:23
leonardri'm specifically wondering if you have any suggestions on how to put the template file somewhere else while still keeping it accessible from utilities/ (i tried pkg_resources loading from canonical.launchpad.templates but it didn't work, probably because c.l.t isn't a module)18:24
leonardrand if you have any ideas on how to get rid of versions_and_descriptions in the namespace--i'm not very good at tales18:24
leonardri'm going to lunch soon, so there's no rush18:24
=== leonardr is now known as leonardr-lunch
=== salgado-afk is now known as salgado
=== leonardr-lunch is now known as leonardr
=== gary-lunch is now known as gary_poster
gary_posterleonardr: I'll look in just a few19:16
leonardrgary, np19:17
salgadoadiroiban, did you get a failure email from ec2 about that branch I submitted for you?19:24
adiroibansalgado: no.19:24
adiroibanbut I don't think ec2 email notification is working19:24
salgadoit is, I got one yesterday19:24
adiroibanthere were some other branches landed on my behalf19:24
adiroibanand I did not get any notification for them19:25
salgadook, then it's possible this one failed because PQM was in testfix mode19:25
* salgado submits once again19:25
adiroibanfor the branches landed by Graham, he has forwarded those emails19:26
adiroibanbut if the branch was not landed19:26
adiroibanit probably means it has errors19:26
salgadogmb, when landing adiroiban's branches, did you use 'ec2 land'?19:28
henningerockstar: Hi!19:29
rockstarhenninge, hi there.19:29
henningerockstar: hey, will you be at UDS?19:29
=== rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
rockstarhenninge, yes, I believe I will.19:30
henningerockstar: cool, you still got my bag?19:30
henninge;-)19:30
rockstarhenninge, I hope I still have your bag.  :)19:30
* henninge imagines rockstar scrambling round his room, looking for it ...19:30
rockstar(we're in the process of remodeling the downstairs)19:31
henningerockstar: if you find it by May, bring it along, please.... ;)19:31
henninge... under the snow.19:31
rockstarIt's probably in the pile of stuff from when we emptied my office.19:31
rockstarhenninge, snow's almost melted actually...  first time since November.19:31
henningerockstar: ah, better than here.19:32
henningeMarch came along and a new coat of snow with it.19:32
henningerockstar: anyway, I do have a branch ready for review, too.19:32
henninge;)19:32
henningebut I am about to leave.19:32
henningerockstar: do you want to give it a go or shall I ask abel in the morning?19:33
rockstarhenninge, if it's complicated, it might have to wait.  If it's simple, I can probably take it.19:33
henningerockstar: complicated depends on if you have been to Wellington, I guess.19:33
rockstarhenninge, UNLESS you write me a novel description of the patch and why it's so complicated.19:33
henningebuild farm stuff19:33
rockstarhenninge, okay, I'll take a look.19:33
henningerockstar: it *does* have a long cover letter.19:33
henningerockstar: cheers19:34
=== henninge changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
henningerockstar: oops, I never pasted this: https://code.edge.launchpad.net/~henninge/launchpad/bug-509557-invoke-pottery/+merge/2117219:46
rockstarhenninge, I already found it.  :)19:46
* henninge makes a mental note to remember to press 'enter' after input19:46
henningerockstar: ok, I am outta here. If you get too confused, drop it ... ;-)19:47
rockstarhenninge, oh, I'll just make comments in the proposal for you to get to in the morning.19:47
henningerockstar: thanks and good night19:48
=== jelmer changed the topic of #launchpad-reviews to: rockstar || reviewing: || queue: [henninge,jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
leonardrgary, it's now a formal review request: https://code.edge.launchpad.net/~leonardr/launchpad/multiversion-apidoc/+merge/2117820:26
gary_posterok20:26
leonardrif you're too busy, maybe flacoste could look?20:26
=== fjlacoste is now known as flacoste
gary_posterleonardr: that's fine.  I will be on call until I finish talking to you20:30
leonardrrockstar, i need to rev lazr.restful, can you review this trivial branch?20:31
leonardrhttp://pastebin.ubuntu.com/393541/20:31
rockstarleonardr, looking20:32
rockstarleonardr, r=me20:33
leonardrgreat20:33
henningerockstar: I am here, btw.20:36
=== NCommander changed the topic of #launchpad-reviews to: rockstar || reviewing: || queue: [henninge,jelmer,NCommander] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
rockstarjelmer, NCommander, I generally don't notice changes to the topic unless the changes in topic are requested first.  :)20:47
jelmerrockstar: oops, sorry :-)20:47
NCommanderrockstar: sorry, first review :-)20:47
rockstarjelmer, NCommander, where are you merge proposals?20:48
jelmerrockstar: https://code.edge.launchpad.net/~jelmer/launchpad/noversion/+merge/2105920:49
NCommanderrockstar: https://code.edge.launchpad.net/~mcasadevall/launchpad/raw_source_changelog/+merge/2117920:49
jelmer(it's a lot of lines, but mostly removing obsolete stuff)20:49
rockstarjelmer, NCommander, ack.  I'll get to them after henninge20:50
jelmerrockstar: thanks!20:50
=== matsubara is now known as matsubara-afk
* kfogel is away: machine fan cleanup; back later22:29
=== salgado is now known as salgado-afk
=== jamalta is now known as jamalta-afk

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