stub | EdwinGrubbs: 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-maisan | Good 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 | ||
noodles775 | stub: 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 |
stub | That is actually normal - client.addFile(foo,...); transaction.abort() leaves a file on disk | 08:26 |
noodles775 | OK - I assumed that the transaction is only aborted in abnormal circumstances. | 08:27 |
stub | Nah - anytime a web request needs to be retried after uploading a file for instance. | 08:27 |
noodles775 | k. 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 | ||
stub | noodles775: test_deleteUnwantedFiles_bug444 or test_delete_unwanted_files_bug444 ? | 08:47 |
stub | noodles775: 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 |
noodles775 | stub: according the the test style guide, I thought the method name should *match* the real method name... | 08:49 |
stub | its a function being tested, not a method, and its name is delete_unwanted_files() | 08:49 |
stub | I'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 |
noodles775 | yep, so I'd use the latter too. | 08:50 |
stub | k | 08:50 |
=== noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
allenap | noodles775: Fancy a simple branch? https://code.edge.launchpad.net/~allenap/launchpad/more-retest/+merge/12709 | 10:49 |
noodles775 | allenap: sure thing! | 10:49 |
allenap | noodles775: Thanks :) | 10:49 |
thumper | deryck: https://code.edge.launchpad.net/~thumper/launchpad/inline-lifecycle-status-edit/+merge/12707 | 10:52 |
noodles775 | allenap: nice - I hadn't seen takewhile before... | 11:04 |
noodles775 | allenap: on line 97, why are you converting to a set rather than a list? | 11:04 |
noodles775 | I 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 | ||
allenap | noodles775: If multiple test logs are passed in (one of the new features), this removes duplicates. | 11:05 |
noodles775 | allenap: then why not convert it back to a sorted list then and there? | 11:05 |
noodles775 | ie., so extract_tests returns a sorted list of unique tests? | 11:06 |
noodles775 | That's great that multiple logs can be used - and that it's now using a buildout template! | 11:07 |
allenap | noodles775: I only want them sorted to display them prettily, otherwise a set is what I want. Not much in it really. | 11:07 |
allenap | noodles775: As in, could be either a set or a list, but it happens to be a set because of de-duplication. | 11:07 |
noodles775 | oh, I didn't see the set features being used anywhere... looking again. | 11:07 |
noodles775 | Ah, gotcha. ok. | 11:07 |
noodles775 | allenap: great, r=me... just note, there's one reference to the old utilities/retest in the usage instructions. | 11:10 |
noodles775 | er, actually, two :) | 11:10 |
allenap | noodles775: 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 |
noodles775 | adeuring: ah great... I get to find out how the new HW character is introduced :) | 12:19 |
mrevell | noodles775: I have a branch fixing the problems we discussed: https://code.launchpad.net/~matthew.revell/launchpad/bugs-439909-439267/+merge/12710 | 12:25 |
noodles775 | mrevell: great, thanks for that! | 12:26 |
noodles775 | mrevell: 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 |
noodles775 | mrevell: and there aren't, but: http://pastebin.ubuntu.com/282928/ | 12:32 |
noodles775 | there's a few other places where we say 'Contact us' but send people to feedback@launchpad.net? | 12:32 |
noodles775 | mrevell: I'll leave it up to you whether you think it's worthwhile making them consistent. | 12:33 |
noodles775 | mrevell: r=me | 12:33 |
mrevell | noodles775: 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 |
noodles775 | Great. | 12:34 |
noodles775 | mrevell: 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 | ||
noodles775 | mrevell: 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 |
mrevell | noodles775: 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 |
noodles775 | hrm... well it was added with r9425 fwiw - check the commit msg there for more details. | 12:39 |
noodles775 | adeuring: gee, that's an unfortunate diff... | 12:43 |
adeuring | noodles775: why? | 12:43 |
noodles775 | adeuring: Nothing wrong with your code, I jut meant the huge segment of HALDevice that diff has assumed has been deleted and added. | 12:44 |
adeuring | noodles775: Ah, OK. | 12:45 |
noodles775 | adeuring: r=me | 12:46 |
adeuring | noodles775: thanks! | 12:46 |
noodles775 | adeuring: 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 |
adeuring | noodles775: I wasn't aware of that option. Thanks for the hint! | 12:49 |
noodles775 | great, np! | 12:49 |
=== Ursinha-afk is now known as Ursinha | ||
maxb | Is that bzr send -r ---> MP thing documented anywhere? | 13:10 |
noodles775 | maxb: bzr help send | 13:14 |
beuno | maxb, https://help.launchpad.net/Code/ReviewEmail%20interface | 13:14 |
noodles775 | at least, that documents that the option exists :) | 13:14 |
noodles775 | ah, even better. | 13:14 |
=== matsubara-afk is now known as matsubara | ||
henninge | Hi! 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 |
henninge | Is 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 |
henninge | jtv: ^ | 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 | ||
barry | noodles775: dang! | 16:44 |
barry | is anybody available to review a 279 line diff? | 16:45 |
noodles775 | barry: there should be some others around soon... :) | 16:45 |
barry | noodles775: 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 | ||
rockstar | barry, leonardr doesn't have a OCR day. | 16:46 |
leonardr | but, barry, if you want me to review something specific, i can | 16:47 |
barry | leonardr: no? you're listed on https://dev.launchpad.net/ReviewerSchedule | 16:47 |
barry | rockstar, leonardr it's xmlrpc and mailman. don't fight over it :) | 16:48 |
* rockstar yells "Not it!" | 16:48 | |
rockstar | :) | 16:48 |
barry | :-D | 16:48 |
rockstar | barry, I'll take it. | 16:48 |
barry | rockstar: thanks! sending the mp now | 16:48 |
=== rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: barry || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
leonardr | barry: that wiki page is probably from when i was being mentored | 16:49 |
barry | leonardr: right. you're not doing ocr anymore? do you want to remove your name from that list? or ic an do it. | 16:50 |
leonardr | barry: i'm doing it now | 16: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 |
barry | leonardr: sounds good. rockstar sorry for the delay. was otp. finishing up the mp now | 17:13 |
rockstar | barry, yeah, I just looked for your proposal and couldn't find it. Was about to bug you. | 17:14 |
barry | rockstar: sent | 17:25 |
bac | rockstar: can i add a MP to your queue? | 17:32 |
rockstar | bac, sure. | 17:32 |
bac | rockstar: cool, still waiting for it to show up | 17:32 |
rockstar | barry, did you request me as reviewer? | 17:35 |
barry | rockstar: oops | 17:35 |
barry | rockstar: done now | 17:36 |
rockstar | barry, I don't see it on lp reviews either. | 17:36 |
barry | rockstar: https://code.edge.launchpad.net/~barry/launchpad/403606-expat/+merge/12734 | 17:36 |
bac | rockstar: https://code.edge.launchpad.net/~bac/launchpad/bug-422128/+merge/12735 | 17:42 |
=== bac changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: barry || queue: [bac] || This channel is logged: http://irclogs.ubuntu.com | ||
rockstar | bac, cool, thanks. | 17:42 |
rockstar | barry, I really only have one comment, and it's a nitpick really. | 17:47 |
bac | rockstar: i need to go get a samwich. you ok with me being awol a bit? | 17:47 |
rockstar | barry, you use "Binarys" on line 59 of the diff. I realize Binary is a class name, but it still looks funny. | 17:47 |
barry | rockstar: how about Binary's ? (kind of still icky) | 17:48 |
rockstar | bac, 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 |
bac | rockstar: that is a sensible plan of action. see you in a bit. | 17:49 |
rockstar | barry, even worse. Bad apostrophe. Maybe "Binary instances" or something. | 17:49 |
barry | rockstar: i like that! | 17:49 |
rockstar | I mean, it could probably stay as it... | 17:49 |
barry | easy to change | 17:49 |
rockstar | barry, other than that, it seems sane to me. | 17:52 |
barry | rockstar: 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 | ||
abentley | rockstar: Could you please review https://code.launchpad.net/~abentley/launchpad/bzr-2.0.0-lp-1/+merge/12738 ? | 18:41 |
rockstar | abentley, yessiree | 18:42 |
abentley | rockstar: Thanks. | 18:42 |
rockstar | abentley, I'm assuming the actual source has landed in download-cache? | 18:43 |
abentley | rockstar: Yes. There's not much involved in that. Commit, push, wait. | 18:44 |
rockstar | abentley, alright, r=me | 18:44 |
abentley | rockstar: Thanks. | 18:45 |
abentley | rockstar: Could you please review https://code.launchpad.net/~abentley/launchpad/diffstat-count/+merge/12720 ? | 18:46 |
bac | thanks rockstar. | 18:46 |
rockstar | bac, you are welcome. | 18:46 |
rockstar | abentley, looks good. r=me | 18:53 |
abentley | rockstar: Thanks. | 18:54 |
abentley | rockstar: Could you please review https://code.launchpad.net/~abentley/launchpad/move-linked-bugs/+merge/12736 ? | 18:54 |
=== Ursinha is now known as Ursinha-afk | ||
rockstar | abentley, it doesn't look like you need the yui-u div around source revisions now. | 18:55 |
abentley | rockstar: quite possible. It's all cargo-culted. | 18:56 |
rockstar | abentley, yeah, I figured. | 18:56 |
rockstar | abentley, so, source-revisions should be wrapped in a yui-g, but that's it. | 19:02 |
=== matsubara-lunch is now known as matsubara | ||
rockstar | abentley, other than that, r=me. | 19:04 |
abentley | rockstar: I'm confused. Change the u to a g? | 19:04 |
rockstar | abentley, yea, and remove the first, or that may result in weirdness. | 19:05 |
rockstar | abentley, yui-g is a row, yui-u is a column | 19:06 |
abentley | rockstar: I miss tables. | 19:07 |
abentley | So do this and I can land it? http://pastebin.ubuntu.com/283191/ | 19:07 |
rockstar | abentley, yeah. Take a look at what the page looks like, but basically, yes, do that. | 19:08 |
abentley | rockstar: Oh great, now I have to push some branches to launchpad.dev | 19:09 |
rockstar | abentley, I just use the factory to make fake revisions. | 19:10 |
rockstar | Oh wait, that won't work for BMPs though, I don't think. | 19:11 |
rockstar | My computer hard locked a bit ago, so if someone was asking for a review, I haven't seen it. Please repost. | 19:41 |
rockstar | Okay, 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 |
rockstar | Also, desktop effects suck. | 19:54 |
leonardr | rockstar, can you do a quick review of https://code.edge.launchpad.net/~leonardr/launchpadlib/release/+merge/12746 ? | 20:29 |
rockstar | leonardr, 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!