| thumper | https://code.launchpad.net/~thumper/launchpad/moar-request-build-logging/+merge/45803 | 01:16 |
|---|---|---|
| StevenK | thumper: There is no Tuesday morning APAC OCR | 03:25 |
| thumper | StevenK: oh well | 03:25 |
| StevenK | Which is a bug, and needs to be fixed. | 03:25 |
| thumper | no rush tehn | 03:25 |
| === mrevell__ is now known as mrevell | ||
| === allenap changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [StevenK, allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | adeuring: Would you be able to review a mostly straightforward branch for a new view? https://code.launchpad.net/~allenap/launchpad/sub-search-ui-bug-656823-4/+merge/45824 | 10:22 |
| adeuring | allenap: sure | 10:22 |
| allenap | adeuring: Thanks. | 10:22 |
| === mrevell_ is now known as mrevell | ||
| adeuring | allenap: r=me | 11:18 |
| === adeuring changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | adeuring: Thank you :) | 11:18 |
| === henninge changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| henninge | adeuring: I will do StevenK's, I already looked at it yesterday. | 11:33 |
| henninge | just in case you were going to pick it up ... | 11:34 |
| adeuring | henninge: cool, thanks | 11:34 |
| === mrevell_ is now known as mrevel | ||
| bac | hi allenap, another go? https://code.edge.launchpad.net/~bac/launchpad/bug-5927/+merge/45403 | 14:19 |
| allenap | bac: Yeah :) | 14:20 |
| === matsubara is now known as matsubara-lunch | ||
| sinzui | mrevell: can you do a UI review of https://code.launchpad.net/~thumper/launchpad/show-recipe-upload-issue/+merge/45804 | 14:54 |
| EdwinGrubbs | leonardr, mars: is one of you oncall? | 15:24 |
| mars | EdwinGrubbs, I will be | 15:24 |
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| mars | just tied up some loose ends | 15:24 |
| EdwinGrubbs | mars: can you review this branch when you get a chance? https://code.launchpad.net/~edwin-grubbs/launchpad/bug-664828-teammembership-delete-timeout/+merge/45805 | 15:25 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: mars || reviewing: - || queue: [Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| mars | EdwinGrubbs, sure | 15:25 |
| === matsubara-lunch is now known as matsubara | ||
| StevenK | henninge: One assert per test makes me very very sad. | 16:08 |
| StevenK | henninge: But I'd argue that each test only touches one concept | 16:09 |
| henninge | StevenK: I said I am not insisting on that. | 16:09 |
| henninge | ;) | 16:09 |
| StevenK | henninge: Right, but it still makes me sad. :-) | 16:09 |
| henninge | but breaking them further down would be better. | 16:09 |
| StevenK | :-( | 16:10 |
| henninge | Sorry to make you sad :( | 16:10 |
| StevenK | Like I say, one concept per test | 16:10 |
| henninge | StevenK: the problem with many asserts in one test is that not all asserts get executed. | 16:11 |
| henninge | StevenK: so, when tests fail, you have to do the "fix-rerrun-fixnext" loop. | 16:11 |
| StevenK | Well, yeah. I've had this fun while writing them. | 16:12 |
| StevenK | henninge: But if you look at say, line 693 of the diff -- I seriously have to split those nine asserts into seperate tests? Which blows out the line count from 20 lines to 70? :-( | 16:14 |
| mwhudson | i don't think that sort of thing is the problem | 16:15 |
| mwhudson | the next test, starting at 714 in the diff is worse, because it's setup, exercise, verify, exercise some more and then verify again | 16:16 |
| mwhudson | oh actually no it's not | 16:16 |
| * mwhudson takes his oar out again | 16:16 | |
| henninge | mwhudson: I agree that what you describe is the worst case but I did not find that here, either. | 16:24 |
| mwhudson | StevenK: did you consider writing a testtools matcher to make these assertions easier to read? | 16:24 |
| henninge | StevenK: *I would* split them up, also because that would be easier to read. I am not fussed about line count here. | 16:25 |
| StevenK | I am, because I have to write it. You just have to read it. :-) | 16:31 |
| henninge | :-) | 16:35 |
| henninge | StevenK: I see you already did a lot of what I asked. Thanks. ;) | 16:35 |
| StevenK | henninge: Have I done enough, though? | 16:36 |
| henninge | StevenK: is that a stray self.setUpBuilds() at the end of thta test file? | 16:36 |
| StevenK | henninge: Er, yes. Well-spotted | 16:36 |
| StevenK | henninge: Fixed locally | 16:37 |
| henninge | StevenK: thanks. r=me ;) | 16:39 |
| StevenK | henninge: Thanks, let me push that fix | 16:40 |
| === bigjools-afk is now known as bigjools | ||
| jtv | hi mars! Can I chuck one on your queue? | 16:54 |
| mars | jtv, sure | 16:54 |
| jtv | mars: ftr it's https://code.launchpad.net/~jtv/launchpad/bug-611217/+merge/45883 | 16:55 |
| === jtv changed the topic of #launchpad-reviews to: On call: mars || reviewing: - || queue: [Edwin, jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | thanks | 16:55 |
| === benji is now known as benji-lunch | ||
| === jtv is now known as jtv-afk | ||
| === gary_poster is now known as gary-lunch | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: - || queue: [Edwin, jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: EdwinGrubbs || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === gary-lunch is now known as gary_poster | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-afk | ||
| EdwinGrubbs | mars: I've responded to your review | 20:50 |
| === mars changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jcsackett | anyone available to review https://code.launchpad.net/~jcsackett/launchpad/shortlist-696913/+merge/45938? | 23:41 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!