/srv/irclogs.ubuntu.com/2009/10/01/#launchpad-reviews.txt

stubEdwinGrubbs: https://code.edge.launchpad.net/~stub/launchpad/librarian-gc/+merge/12695 if you are still there.03:51
=== stub changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
al-maisanGood morning!06:38
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: stub || queue: [] || This channel is logged: http://irclogs.ubuntu.com
noodles775stub: hi! Just looking at your MP... would it be worth in addition creating a separate condition to log a warning for this situation where the file exists on disk but not in the db?08:24
stubThat is actually normal - client.addFile(foo,...); transaction.abort() leaves a file on disk08:26
noodles775OK - I assumed that the transaction is only aborted in abnormal circumstances.08:27
stubNah - anytime a web request needs to be retried after uploading a file for instance.08:27
noodles775k. great.08:27
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com
stubnoodles775: test_deleteUnwantedFiles_bug444 or test_delete_unwanted_files_bug444 ?08:47
stubnoodles775: The former matches the existing test method names more closely, the latter is correct as the function being tested is called delete_unwanted_files()08:47
noodles775stub: according the the test style guide, I thought the method name should *match* the real method name...08:49
stubits a function being tested, not a method, and its name is delete_unwanted_files()08:49
stubI'd probably go for the latter option myself, which I think is correct per the guide, and stuff the existing naming in that file (circa 2005 IIRC)08:50
noodles775yep, so I'd use the latter too.08:50
stubk08:50
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
allenapnoodles775: Fancy a simple branch? https://code.edge.launchpad.net/~allenap/launchpad/more-retest/+merge/1270910:49
noodles775allenap: sure thing!10:49
allenapnoodles775: Thanks :)10:49
thumperderyck: https://code.edge.launchpad.net/~thumper/launchpad/inline-lifecycle-status-edit/+merge/1270710:52
noodles775allenap: nice - I hadn't seen takewhile before...11:04
noodles775allenap: on line 97, why are you converting to a set rather than a list?11:04
noodles775I mean, test output will not have duplicates...?11:05
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: allenap || queue: [] || This channel is logged: http://irclogs.ubuntu.com
allenapnoodles775: If multiple test logs are passed in (one of the new features), this removes duplicates.11:05
noodles775allenap: then why not convert it back to a sorted list then and there?11:05
noodles775ie., so extract_tests returns a sorted list of unique tests?11:06
noodles775That's great that multiple logs can be used - and that it's now using a buildout template!11:07
allenapnoodles775: I only want them sorted to display them prettily, otherwise a set is what I want. Not much in it really.11:07
allenapnoodles775: As in, could be either a set or a list, but it happens to be a set because of de-duplication.11:07
noodles775oh, I didn't see the set features being used anywhere... looking again.11:07
noodles775Ah, gotcha. ok.11:07
noodles775allenap: great, r=me... just note, there's one reference to the old utilities/retest in the usage instructions.11:10
noodles775er, actually, two :)11:10
allenapnoodles775: Ah, good spot, thanks :)11:11
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
=== jtv is now known as jtv-afk
adeuring noodles775: could you please review a sequel of my last MP: https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-refactor-haldevice-2/+merge/12713 ?12:19
noodles775adeuring: ah great... I get to find out how the new HW character is introduced :)12:19
mrevellnoodles775: I have a branch fixing the problems we discussed: https://code.launchpad.net/~matthew.revell/launchpad/bugs-439909-439267/+merge/1271012:25
noodles775mrevell: great, thanks for that!12:26
noodles775mrevell: doing yours first as it's tiny :), I was just checking to see if there were any tests that might check the url for the contact-us link...12:32
noodles775mrevell: and there aren't, but: http://pastebin.ubuntu.com/282928/12:32
noodles775there's a few other places where we say 'Contact us' but send people to feedback@launchpad.net?12:32
noodles775mrevell: I'll leave it up to you whether you think it's worthwhile making them consistent.12:33
noodles775mrevell: r=me12:33
mrevellnoodles775: Yeah. The first of those, main-template, I believe is an over hang from the old design. I'll fix the other links. Thanks.12:33
noodles775Great.12:34
noodles775mrevell: and, of course, run it through ec2 first as there could be a test somewhere looking for the contact us url/href etc.12:34
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com
noodles775mrevell: out of interest, did you find out what's behind the new version.txt file (ie. should it be updated automatically or at least as part of the rollout process?)12:36
mrevellnoodles775: sinzui didn't know where it had come from, but I'm going to ask beuno. I'm assuming it'll need a manual update.12:37
noodles775hrm... well it was added with r9425 fwiw - check the commit msg there for more details.12:39
noodles775adeuring: gee, that's an unfortunate diff...12:43
adeuringnoodles775: why?12:43
noodles775adeuring: Nothing wrong with your code, I jut meant the huge segment of HALDevice that diff has assumed has been deleted and added.12:44
adeuringnoodles775: Ah, OK.12:45
noodles775adeuring: r=me12:46
adeuringnoodles775: thanks!12:46
noodles775adeuring: did you try using the revision spec '-r x..y' with bzr send so that the colour-coded diff on the mp is just the revisions you want? If not, might be good for the third installment :)12:48
adeuringnoodles775: I wasn't aware of that option. Thanks for the hint!12:49
noodles775great, np!12:49
=== Ursinha-afk is now known as Ursinha
maxbIs that bzr send -r ---> MP thing documented anywhere?13:10
noodles775maxb: bzr help send13:14
beunomaxb, https://help.launchpad.net/Code/ReviewEmail%20interface13:14
noodles775at least, that documents that the option exists :)13:14
noodles775ah, even better.13:14
=== matsubara-afk is now known as matsubara
henningeHi! I want to use a TAL formatter in a view and I found out I can do it this way: https://pastebin.canonical.com/22831/15:18
henningeIs that the right way to do it? Or should I go through the registration somehow? I remember seeing an example once that was different ...15:19
henningejtv: ^15:19
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
* noodles775 finishes reviewing for the day.16:31
=== noodles775 changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
barrynoodles775: dang!16:44
barryis anybody available to review a 279 line diff?16:45
noodles775barry: there should be some others around soon... :)16:45
barrynoodles775: i guess not cprov-afk though :(16:46
* barry waits for rockstar and leonardr 16:46
=== rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
rockstarbarry, leonardr doesn't have a OCR day.16:46
leonardrbut, barry, if you want me to review something specific, i can16:47
barryleonardr: no? you're listed on https://dev.launchpad.net/ReviewerSchedule16:47
barryrockstar, leonardr it's xmlrpc and mailman.  don't fight over it :)16:48
* rockstar yells "Not it!"16:48
rockstar:)16:48
barry:-D16:48
rockstarbarry, I'll take it.16:48
barryrockstar: thanks!  sending the mp now16:48
=== rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: barry || queue: [] || This channel is logged: http://irclogs.ubuntu.com
leonardrbarry: that wiki page is probably from when i was being mentored16:49
barryleonardr: right.  you're not doing ocr anymore?  do you want to remove your name from that list?  or ic an do it.16:50
leonardrbarry: i'm doing it now16:50
leonardr(removing my name--i'm not doing ocr, i went through mentoring because it was ridiculous to review lazr.* branches and then have to get someone else to do a 'real' review)16:51
barryleonardr: sounds good.  rockstar sorry for the delay.  was otp.  finishing up the mp now17:13
rockstarbarry, yeah, I just looked for your proposal and couldn't find it.  Was about to bug you.17:14
barryrockstar: sent17:25
bacrockstar: can i add a MP to your queue?17:32
rockstarbac, sure.17:32
bacrockstar: cool, still waiting for it to show up17:32
rockstarbarry, did you request me as reviewer?17:35
barryrockstar: oops17:35
barryrockstar: done now17:36
rockstarbarry, I don't see it on lp reviews either.17:36
barryrockstar: https://code.edge.launchpad.net/~barry/launchpad/403606-expat/+merge/1273417:36
bacrockstar: https://code.edge.launchpad.net/~bac/launchpad/bug-422128/+merge/1273517:42
=== bac changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: barry || queue: [bac] || This channel is logged: http://irclogs.ubuntu.com
rockstarbac, cool, thanks.17:42
rockstarbarry, I really only have one comment, and it's a nitpick really.17:47
bacrockstar: i need to go get a samwich.  you ok with me being awol a bit?17:47
rockstarbarry, you use "Binarys" on line 59 of the diff.  I realize Binary is a class name, but it still looks funny.17:47
barryrockstar: how about Binary's ? (kind of still icky)17:48
rockstarbac, if you're not here when I get to it, I'll note my questions in the MP and you can address them when you get back.17:48
bacrockstar: that is a sensible plan of action.  see you in a bit.17:49
rockstarbarry, even worse.  Bad apostrophe.  Maybe "Binary instances" or something.17:49
barryrockstar: i like that!17:49
rockstarI mean, it could probably stay as it...17:49
barryeasy to change17:49
rockstarbarry, other than that, it seems sane to me.17:52
barryrockstar: thanks!17:53
=== matsubara is now known as matsubara-lunch
=== rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
abentleyrockstar: Could you please review https://code.launchpad.net/~abentley/launchpad/bzr-2.0.0-lp-1/+merge/12738 ?18:41
rockstarabentley, yessiree18:42
abentleyrockstar: Thanks.18:42
rockstarabentley, I'm assuming the actual source has landed in download-cache?18:43
abentleyrockstar: Yes.  There's not much involved in that.  Commit, push, wait.18:44
rockstarabentley, alright, r=me18:44
abentleyrockstar: Thanks.18:45
abentleyrockstar: Could you please review https://code.launchpad.net/~abentley/launchpad/diffstat-count/+merge/12720 ?18:46
bacthanks rockstar.18:46
rockstarbac, you are welcome.18:46
rockstarabentley, looks good.  r=me18:53
abentleyrockstar: Thanks.18:54
abentleyrockstar: Could you please review https://code.launchpad.net/~abentley/launchpad/move-linked-bugs/+merge/12736 ?18:54
=== Ursinha is now known as Ursinha-afk
rockstarabentley, it doesn't look like you need the yui-u div around source revisions now.18:55
abentleyrockstar: quite possible.  It's all cargo-culted.18:56
rockstarabentley, yeah, I figured.18:56
rockstarabentley, so, source-revisions should be wrapped in a yui-g, but that's it.19:02
=== matsubara-lunch is now known as matsubara
rockstarabentley, other than that, r=me.19:04
abentleyrockstar: I'm confused.  Change the u to a g?19:04
rockstarabentley, yea, and remove the first, or that may result in weirdness.19:05
rockstarabentley, yui-g is a row, yui-u is a column19:06
abentleyrockstar: I miss tables.19:07
abentleySo do this and I can land it? http://pastebin.ubuntu.com/283191/19:07
rockstarabentley, yeah.  Take a look at what the page looks like, but basically, yes, do that.19:08
abentleyrockstar: Oh great, now I have to push some branches to launchpad.dev19:09
rockstarabentley, I just use the factory to make fake revisions.19:10
rockstarOh wait, that won't work for BMPs though, I don't think.19:11
rockstarMy computer hard locked a bit ago, so if someone was asking for a review, I haven't seen it.  Please repost.19:41
rockstarOkay, so, I have to say this again just in case I missed something when my system locked again.  Did anyone request a review from me?19:54
rockstarAlso, desktop effects suck.19:54
leonardrrockstar, can you do a quick review of https://code.edge.launchpad.net/~leonardr/launchpadlib/release/+merge/12746 ?20:29
rockstarleonardr, sure.20:29
=== Ursinha-afk is now known as Ursinha
=== rockstar changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
=== Ursinha is now known as Ursinha-afk
=== matsubara is now known as matsubara-afk

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