| === abentley1 is now known as abentley | ||
| === gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| === mrevell is now known as mrevell-lunch | ||
| bac | hi gmb. can i put a review on your queue? | 12:34 |
|---|---|---|
| bac | noodles775: can you do a UI review for me? | 12:35 |
| noodles775 | bac: sure | 12:35 |
| bac | noodles775: thanks, i just added you to https://code.edge.launchpad.net/~bac/launchpad/bug-436980-mugshots/+merge/13250 | 12:35 |
| gmb | bac: Sure. | 12:36 |
| bac | gmb: cool. it's the one i just posted | 12:37 |
| gmb | Righto. | 12:37 |
| === gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: bac || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| === ursula is now known as Ursinha | ||
| gmb | bac: Looks good. r=me. | 12:47 |
| === gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| bac | thanks gmb | 12:47 |
| === mrevell-lunch is now known as mrevell | ||
| noodles775 | bac: r=me (one suggestion on the MP). | 13:00 |
| noodles775 | Looks great! | 13:00 |
| bac | thanks noodles775. do i have to have beuno give it a look too? | 13:00 |
| noodles775 | bac: if he's around, it'd be great (for my learning). beuno ? | 13:01 |
| matsubara | ! | 13:22 |
| matsubara | oops | 13:22 |
| beuno | hi bac, noodles775 | 13:30 |
| noodles775 | hi beuno :) | 13:30 |
| noodles775 | beuno: I'm just heading for lunch, but it'd be great if you could any comments you have to the MP, thanks! | 13:34 |
| beuno | noodles775, bug-436980-mugshots? | 13:34 |
| noodles775 | beuno: yep | 13:34 |
| beuno | noodles775, on it | 13:34 |
| beuno | bac, noodles775, I'm suprised how many things I managed to squeeze out of that page, but, anyway, reviewed | 13:57 |
| bac | beuno: squeeze? | 13:57 |
| beuno | bac, long review for such a small and fairly unused page | 13:58 |
| bac | beuno: it already uses 32 | 13:58 |
| bac | beuno: so much for not reading the merge proposal! :) | 13:58 |
| beuno | bac, as you can see, I follow my guidelines to the letter | 13:58 |
| beuno | ah, 8 en dev | 13:59 |
| beuno | :) | 13:59 |
| === bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| === ursula is now known as Ursinha | ||
| bac | hi noodles775 | 15:04 |
| noodles775 | Hi bac | 15:06 |
| bac | noodles775: thanks for the CSS suggestion. much nicer. | 15:06 |
| noodles775 | bac: np! glad it was useful. | 15:06 |
| bac | noodles775: were you recommending changing to "Member photos" for the label, which affects h1, breadcrumbs, and page title? that's what i've done and it seems much better. | 15:06 |
| noodles775 | bac: yes, that seems to be the consensus on the lp-dev email discussion I sent out last wednesday... (I personally prefer the h1 to be different from the leaf breadcrumb, but that's beside the point :) ). | 15:08 |
| === bac changed the topic of #launchpad-reviews to: on call: gmb, bac || reviewing: -,mrevell || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| bac | hi mrevell | 15:37 |
| === gmb changed the topic of #launchpad-reviews to: on call: bac || reviewing: mrevell || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| * gmb -> afk for a while | 15:38 | |
| mrevell | bac hi, on a call atm | 15:38 |
| bac | see ya gmb | 15:38 |
| bac | mrevell: ok, i'm looking at your editpgp review. give me a shout when you have a sec. | 15:38 |
| abentley | bac: Could you please review https://code.launchpad.net/~abentley/launchpad/reuse-refactor/+merge/13283 ? | 15:52 |
| bac | abentley: sure | 15:53 |
| === bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: mrevell || queue: [abentley] || This channel is logged: http://irclogs.ubuntu.com | ||
| bac | abentley: you aren't using the refactored code anywhere yet, right? so the only demo is bug comments? | 15:58 |
| abentley | bac: Right. | 15:58 |
| abentley | bac: I'm just polishing up the branch that uses it for merge proposals. | 15:58 |
| bac | abentley: ok | 15:59 |
| bac | hi rockstar | 16:11 |
| bac | abentley: this is unrelated to your branch, but do merge proposals now update the diff when new revisions of the branch are pushed? i think i just saw that happen on one of my MPs. | 16:14 |
| abentley | bac: Yes, they now do. | 16:14 |
| bac | abentley: that seems strange to me. you lose the historical context for the review. | 16:15 |
| abentley | I resisted it at first, too, but users demanded it. | 16:16 |
| abentley | bac: We will be linking reviews to the particular diff they relate to. | 16:16 |
| bac | abentley: an incremental added to the MP would be fantastic. | 16:16 |
| bac | abentley: ok. glad to know my eyes weren't deceiving me. | 16:16 |
| abentley | Incremental diffs have been requested, too. We would like to do them, but they are not easy to add, because if there was a merge, you don't want to include changes from that merge. | 16:20 |
| === EdwinGrubbs is now known as Edwin-afk | ||
| rockstar | bac, hi. | 16:40 |
| bac | hi rockstar -- i was going to ask you about MP diffs but had the chat with abentley. | 16:40 |
| rockstar | bac, great. I didn't realize that my IRC client wasn't connected until just now. | 16:41 |
| rockstar | (no wonder I was being so productive) | 16:41 |
| mrevell | hi bac, off my call now | 16:42 |
| bac | abentley: r=bac. wonderful change. | 16:45 |
| bac | hi mrevell. i had some questions about your editgpg branch | 16:45 |
| mrevell | bac: Ah, great, I hope I can answer them | 16:45 |
| bac | mrevell: first, not your fault,but i saw a typo in that help section. "a keyservers" i think it was. it'll be obvious when you see it. | 16:46 |
| mrevell | bac: I'll fix that (almost certainly is fault). Thanks. | 16:46 |
| bac | mrevell: more to your content, having the note saying it'll take a while followed by the next set of instructions beginning with "now (do this)" reads funny. | 16:47 |
| abentley | bac: Thanks! | 16:47 |
| bac | i wonder if the contents of the note shouldn't just be moved into the next step and worded so that it makes sense as a next step | 16:47 |
| mrevell | bac: Hmm, yes, good suggestion. I'll do that. | 16:47 |
| bac | mrevell: ok, great. i'll put that in your MP and approve it now. thanks for the branch. | 16:48 |
| abentley | bac: What would you change the inline comment to? | 16:48 |
| mrevell | thank you bac | 16:49 |
| bac | abentley: just move it up to precede the line. | 16:49 |
| abentley | bac: Will do. | 16:49 |
| bac | abentley: i like those little end-of-the-line comments myself... | 16:49 |
| bac | mrevell: also, it might be good if you were to reword the error message one gets when trying to import a GPG key that isn't yet available | 16:54 |
| bac | mrevell: right now it suggests submitting directly to our keyserver, but even that has a delay | 16:54 |
| mrevell | Ah, right. thanks bac, I'll take a look at that. | 16:55 |
| bac | mrevell: it's in c/l/browser/logintoken.py | 16:55 |
| mrevell | thanks bac | 16:56 |
| bac | mrevell: oops, it's in lp/registry/templates/person-editpgpkey.pt | 16:56 |
| === bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: mrevell || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| mrevell | ah, that sounds familiar, thanks bac | 17:02 |
| === bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| === salgado is now known as salgado-lunch | ||
| === beuno is now known as beuno-lunch | ||
| === matsubara is now known as matsubara-lunch | ||
| === salgado-lunch is now known as salgado | ||
| bac | bigjools: your review is done | 18:45 |
| === beuno-lunch is now known as beuno | ||
| bac | salgado: i will if you ask over here! | 18:48 |
| adeuring | bac: could you please review this branch: https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-class-udev-device-4/+merge/13300 ? | 18:49 |
| === bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: salgado || queue: [abel] || This channel is logged: http://irclogs.ubuntu.com | ||
| bac | adeuring: i will. add me to the MP? | 18:51 |
| adeuring | bac: thanks, done | 18:52 |
| achuni | hi, is it normal to get mails about Launchpad Internal Error when notifying people about proposals to merge? | 19:02 |
| achuni | (it was a proposal to merge in to Launchpad itself, which is why I ask here) | 19:03 |
| === matsubara-lunch is now known as matsubara | ||
| salgado | achuni, I just got one. can you give me the OOPS ID of yours? | 19:06 |
| achuni | salgado: OOPS-1382MPCJ5 | 19:07 |
| salgado | abentley, around? I just got https://lp-oops.canonical.com/oops.py/?oopsid=1382CMP1 when submitting a merge proposal | 19:07 |
| salgado | and achuni is a completely different one, it seems | 19:08 |
| achuni | anybody up for reviewing https://code.launchpad.net/~canonical-isd-hackers/launchpad/sso-new-branding/+merge/13304 ? | 19:12 |
| bigjools | thanks bac! | 19:27 |
| abentley | salgado: That is bug #436325 | 19:46 |
| mup | Bug #436325: Diffstat generation chokes on binaries (and others) <oops> <Launchpad Bazaar Integration:Triaged by abentley> <https://launchpad.net/bugs/436325> | 19:46 |
| salgado | abentley, and did you see achuni's OOPS? OOPS-1382MPCJ5 | 19:47 |
| bac | achuni: i'll review it in a bit | 19:48 |
| abentley | salgado: That's a bug introduced by thumper. I'm not sure whether it has been reported. | 19:48 |
| === bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: abel || queue: [achuni] || This channel is logged: http://irclogs.ubuntu.com | ||
| salgado | achuni, ^ | 19:49 |
| achuni | abentley: should I report? | 19:49 |
| achuni | abentley: thanks for looking in to it | 19:49 |
| abentley | achuni: up to to you. | 19:50 |
| bac | adeuring: for pci_ids the docstring says you return a tuple but you return a list | 19:53 |
| adeuring | bac: argh... fiexd. | 19:54 |
| bac | adeuring: i think returning the tuple would be best | 19:54 |
| adeuring | bac: hmm, that requires another tuple([]), or I replace the list comprehension | 19:55 |
| bac | adeuring: can't imagine that call to tuple is expensive. | 19:55 |
| adeuring | bac: well, OK, I'll do that | 19:55 |
| bac | adeuring: abel at line 74 can you compare against UDEV_ROOT_PATH instead of the string? | 19:56 |
| adeuring | bac: sure | 19:56 |
| bac | adeuring: at line 77 I was unsure what the value would be if both were None. I now see that 'None or None' is None. (conceivably it could be False.) anyway since that construct is confusing perhaps you can write it in a more readable manner. | 20:00 |
| adeuring | bac: sure | 20:01 |
| adeuring | bac: diff so far: http://pastebin.ubuntu.com/292561/ | 20:03 |
| bac | adeuring: I'd like to make a suggestion to see what you think. | 20:04 |
| abentley | bac: Could you please review https://code.edge.launchpad.net/~abentley/launchpad/bmp-inline/+merge/13299 ? | 20:04 |
| bac | abentley: sure. please add me to the MP | 20:04 |
| abentley | bac: I don't want to add you the the mp, as you are a member of launchpad code reviewers, which is already listed. Could you please claim the review instead? | 20:05 |
| bac | adeuring: a lot of your code is based on selecting between vendor and product. if your methods were returning dictionaries with those as the keys you could get rid of a lot of the conditionals and it would be much cleaner. | 20:05 |
| bac | abentley: oh, ok | 20:05 |
| achuni | bac: thanks :) | 20:05 |
| adeuring | bac: nice idea! | 20:06 |
| bac | abentley: except i don't see a "Claim" button any more! | 20:06 |
| bac | adeuring: ok, i'll make that broad suggestion in the review and let you figure out the details | 20:07 |
| adeuring | bac: sure, np | 20:07 |
| abentley | bac: Me neither, so I've reassigned it to you instead. | 20:07 |
| bac | abentley: thanks | 20:07 |
| === bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: abel || queue: [achuni, abentley] || This channel is logged: http://irclogs.ubuntu.com | ||
| abentley | rockstar: I can has UI review of https://code.edge.launchpad.net/~abentley/launchpad/bmp-inline/+merge/13299 ? | 20:12 |
| achuni | bac: If you've still got time left after that, I've just proposed a merge for a matching canonical-identity-provider branch, ui approved already. | 20:13 |
| bac | achuni: i'm here all afternoon! | 20:13 |
| achuni | oh good, because it's a biggie :P | 20:13 |
| achuni | bac: https://code.launchpad.net/~canonical-isd-hackers/canonical-identity-provider/wip/+merge/13309 | 20:14 |
| bac | achuni: how big? | 20:14 |
| bac | achuni: please subscribe me (bac) to that branch as i cannot see the MP | 20:15 |
| achuni | on it | 20:15 |
| achuni | bac: done | 20:16 |
| bac | achuni: is that MP accurate? 3200+ lines? on launchpad we have a limit of 800 lines -- after that we strongly consider doing multiple branches. | 20:18 |
| achuni | bac: I'm afraid it is... we weren't aware of the limit | 20:19 |
| achuni | bac: I guess I'll split this in to a few branches and come back | 20:20 |
| bac | achuni: ok, i'll do the review if i have time but will favor any smaller branches that come in. we've agreed that's the way the on-call reviews should work to ensure the highest throughput. | 20:20 |
| achuni | thanks bac. understood, and I promise future merges will be more reasonable | 20:22 |
| beuno | bac, it's mostly because it's a multi-person branch | 20:22 |
| beuno | and I've been holding it back because a lot of the UI stuff | 20:22 |
| beuno | I think there's like 4 or 5 different people who worked on it | 20:22 |
| beuno | so it's partially my fault :) | 20:22 |
| bac | beuno: np. it is what it is. | 20:23 |
| === bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: achuni || queue: [abentley, achuni et al] || This channel is logged: http://irclogs.ubuntu.com | ||
| === bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: achuni || queue: [achuni et al] || This channel is logged: http://irclogs.ubuntu.com | ||
| abentley | rockstar: I can has review of https://code.edge.launchpad.net/~abentley/launchpad/bmp-inline/+merge/13299 | 20:39 |
| bac | hi abentley, this MP from achuni has no diff generated: https://code.edge.launchpad.net/~canonical-isd-hackers/launchpad/sso-new-branding/+merge/13304 | 20:42 |
| bac | thoughts? | 20:42 |
| abentley | bac: That's because diff generation oopsed. | 20:46 |
| bac | abentley: ok. | 20:46 |
| abentley | bac: That's the oops you pointed out to me earlier. | 20:47 |
| bac | abentley: not me. | 20:48 |
| abentley | bac: Sorry, salgado reported it. | 20:48 |
| abentley | bac: https://lp-oops.canonical.com/oops.py/?oopsid=OOPS-1382MPCJ5 | 20:49 |
| rockstar | abentley, looking now. | 20:56 |
| abentley | thumper: Chat? | 21:00 |
| bac | hi sinzui | 21:03 |
| sinzui | hi bac | 21:03 |
| adeuring | bac: reply sent | 21:09 |
| thumper | abentley: sure | 21:11 |
| bac | adeuring: s/rott/root | 21:33 |
| bac | adeuring: actually root_device_ids doesn't need to be a method. it can just be a property, i.e. root_device_ids = {'vendor'...} | 21:35 |
| bac | abentley, rockstar, thumper: whoever moved the edit icon for MP status back to being in-line is my new hero. | 21:39 |
| rockstar | bac, that was me. | 21:39 |
| thumper | bac: I'll be making is javascript soon | 21:40 |
| bac | rockstar: thanks! | 21:40 |
| bac | thumper: and that'll be super sweet | 21:40 |
| bac | rockstar: oh, i'm in on your legal defense fund for any face punching you do. | 21:40 |
| bac | well, not any random punching, just the one you promised | 21:40 |
| rockstar | bac, :) | 21:48 |
| adeuring | bac: no, root_device_ids needs to access self.dmi, which is only defined once the constructor has been called, so we need a method | 21:49 |
| bac | adeuring: ok | 21:49 |
| adeuring | bac: type is fixed | 21:49 |
| === bac changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| === salgado is now known as salgado-afk | ||
| === salgado is now known as salgado-afk | ||
| === matsubara is now known as matsubara-afk | ||
| === flacoste is now known as flacoste_afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!