| === EdwinGrubbs 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 | ||
| wgrant | Anyone around to review a tiny regression fix? | 04:40 |
|---|---|---|
| wgrant | https://code.launchpad.net/~wgrant/launchpad/bug-522800-getFileByName/+merge/43159 | 04:40 |
| * mwhudson takes a look | 04:40 | |
| === jcsackett changed the topic of #launchpad-reviews to: On call: jcsackett || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-lunch | ||
| === matsubara-lunch is now known as matsubara | ||
| === salgado is now known as salgado-lunch | ||
| === henninge changed the topic of #launchpad-reviews to: On call: jcsackett || Reviewing: - || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| henninge | Hi jcsackett! | 16:55 |
| henninge | I have a branch that is actually quite simple. | 16:55 |
| jcsackett | hello, henninge. i would be happy to look at it. | 16:55 |
| jcsackett | link? | 16:55 |
| henninge | But it contains a sample data change that makes the diff look big. | 16:56 |
| henninge | https://code.edge.launchpad.net/~henninge/launchpad/recife-611668-fix-tests/+merge/43236 | 16:56 |
| henninge | jcsackett: thanks | 16:56 |
| jcsackett | henninge: you're welcome. | 16:56 |
| === jcsackett changed the topic of #launchpad-reviews to: On call: jcsackett || Reviewing: henninge || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jcsackett | henninge: in pofile.txt did you move the bit "msgid = potmsgset.msgid_singular.msgid..." out of the loop just to speed things up, since it appears to be a value that won't change in iteration? | 17:02 |
| jcsackett | or was there another purpose? | 17:02 |
| henninge | no, just because there was no reason for it to be in the loop | 17:03 |
| henninge | It may even speed up the loop, yes. ;) | 17:03 |
| jcsackett | henninge: cool, thanks. | 17:03 |
| henninge | The test was failing because of the output from the format string further down. | 17:04 |
| henninge | The old format produced trailing spaces but I don't understand why that worked before. | 17:04 |
| henninge | jcsackett: I have to run out right now but will be back in about an hour. Sorry. | 17:05 |
| jcsackett | henninge: no problems. | 17:05 |
| === salgado-lunch is now known as salgado | ||
| === deryck is now known as deryck[lunch] | ||
| EdwinGrubbs | jcsackett: I've added my branch to the queue: https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-676966-disallow-merging-people-with-ppas/+merge/43116 | 17:19 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: jcsackett || Reviewing: henninge || queue: [Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jcsackett | EdwinGrubbs: okay. | 17:20 |
| === jcsackett changed the topic of #launchpad-reviews to: On call: jcsackett || Reviewing: Edwin || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === benji is now known as benji-lunch | ||
| === deryck[lunch] is now known as deryck | ||
| jcsackett | EdwinGrubbs: are you planning on fixing the lint errors that you put in the MP? | 18:33 |
| EdwinGrubbs | jcsackett: yes, except for the one where it complains about two blank lines because of a comment. | 18:34 |
| jcsackett | EdwinGrubbs: yeah, i was ignoring that one. just confused b/c i generally clean up lint before submitting, but either way works as long as they'll get fixed before merging. | 18:35 |
| henninge | jcsackett: Hi | 18:38 |
| jcsackett | henninge: hey there. | 18:38 |
| henninge | jcsackett: There is a prerequisite branch, as mentioned in the MP | 18:39 |
| jcsackett | henninge: yes, i got that. i didn't have failing tests. :-/ | 18:39 |
| henninge | hm | 18:39 |
| EdwinGrubbs | jcsackett: sometimes, reviewers don't like it in the mp diff, because it makes it harder to understand the real changes. | 18:39 |
| jcsackett | EdwinGrubbs: fair point. | 18:39 |
| jcsackett | henninge: i will look again after reviewing Edwin's branch. there is nothing else but the recife branch, the prereq listed, and yours to consider, correct? | 18:40 |
| henninge | jcsackett: actually, there are three other branches before that but I thought they'd be included in the one branch (because it depends on them) | 18:41 |
| henninge | jcsackett: the other problem is that the original branches were started against db-devel | 18:42 |
| henninge | but now the the merge is into recife. | 18:42 |
| henninge | because that was backed out of db-devel again. | 18:42 |
| jcsackett | hm. | 18:42 |
| jcsackett | henninge: what branch were you looking in that had the errors? | 18:43 |
| henninge | jcsackett: db-devel-bug-611668-filtermethods-2 merged into recife | 18:43 |
| jcsackett | henninge: okay, that is what i thought i was looking at, but i will rebuild it and double check. | 18:43 |
| jcsackett | btw, henninge: where does the name "recife" come from? | 18:44 |
| henninge | jcsackett: it's the place of the sprint where this branch was started ... in March :/ | 18:45 |
| henninge | Recife, Brazil | 18:45 |
| jcsackett | henninge: ah. cool. | 18:45 |
| jcsackett | EdwinGrubbs: there are a few comments on your MP. | 19:06 |
| === jcsackett changed the topic of #launchpad-reviews to: On call: jcsackett || Reviewing: henninge || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === benji-lunch is now known as benji | ||
| jcsackett | henninge: taking another look now. | 19:08 |
| henninge | jcsackett: I think I know what is going on here | 19:10 |
| jcsackett | henninge: hit me. | 19:10 |
| henninge | does pofile.txt fail for you? | 19:11 |
| henninge | That is the test that was originally failing for me. | 19:11 |
| jcsackett | henninge: tell you in a second, building bin/test for your branch now. | 19:11 |
| henninge | I fixed that by updating the sample data, as described. | 19:11 |
| jcsackett | rather, building bin/test for recife now. | 19:12 |
| henninge | After that, the other two tests failed because the ordering of the translationsmessages changed. | 19:12 |
| henninge | They were ordered by "date_last_touched" | 19:12 |
| henninge | and two had been touched by the migration script. | 19:13 |
| henninge | So I "fixed" that by manually setting the "date_last_touched" back to what it was before (in the sampledata) instead of changing the order in the test. | 19:14 |
| henninge | So it's correct, the two tests translationsperson.txt and xx-person-activity.txt will pass before and after branch. | 19:15 |
| henninge | So, only pofile.txt is being fixed by this branch, the others were just "not broken"... | 19:15 |
| EdwinGrubbs | jcsackett: thanks for the review | 19:17 |
| jcsackett | henninge: that seems to be the case (coupled with me possibly messing up purity of branches when checking pofile, which didn't fail on recife before). | 19:18 |
| jcsackett | but carefully rebuilding the state of the branch before your MP does indeed show pofile failing. | 19:18 |
| jcsackett | henninge: r=me; i am mentoring so sinzui will need to follow up. i have already requested a review from him. | 19:20 |
| === jcsackett changed the topic of #launchpad-reviews to: On call: jcsackett || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| henninge | jcsackett: thanks ;-) | 19:20 |
| jcsackett | henninge: your welcome. sorry i evidently goofed on checking the tests. :-P | 19:21 |
| henninge | jcsackett: and I mislead you by implying that these tests were failing. | 19:28 |
| === EdwinGrubbs is now known as Edwin-afk2 | ||
| === matsubara is now known as matsubara-afk | ||
| === salgado is now known as salgado-afk | ||
| bac | jcsackett: time for a quick review? | 21:25 |
| jcsackett | bac: sure. | 21:26 |
| bac | https://code.edge.launchpad.net/~bac/launchpad/bug-662994/+merge/43299 | 21:26 |
| === jcsackett changed the topic of #launchpad-reviews to: On call: jcsackett || Reviewing: bac || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jcsackett | bac: bit of a delay; the diff has not yet appeared. :-P | 21:27 |
| bac | jcsackett: it is there now | 21:27 |
| jcsackett | bac: huzzah. | 21:28 |
| jcsackett | bac: are there no unit tests around this the review view? | 21:33 |
| bac | jcsackett: i didn't find any. i held my nose and updated the story | 21:35 |
| jcsackett | hrm. i'm tempted to file a bug against the lack of actual unit tests in place of stories... | 21:36 |
| bac | jcsackett: i can defer this one and create the unit tests | 21:37 |
| jcsackett | bac: i would buy you a beer if you did that. :-) | 21:37 |
| bac | ok | 21:38 |
| jcsackett | i like the code though. looks like a good branch. | 21:38 |
| jcsackett | you want me to still claim the MP, or leave it till later? | 21:38 |
| jcsackett | if you want me to go ahead and claim it, i'll happily take a look at it whenever you're done. | 21:39 |
| jcsackett | bac ^ | 21:39 |
| bac | i'll just retract it | 21:39 |
| jcsackett | bac: okay. thanks for putting tests in place: you're a gentleman and a scholar. | 21:40 |
| bac | bah | 21:40 |
| === jcsackett 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 | is there anyone available to review: https://code.edge.launchpad.net/~jcsackett/launchpad/membership-status-matters-676494/+merge/43303 | 22:46 |
| jcsackett | it's a test + a small change to a sql query | 22:46 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!