| === 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 | ||
| === jelmer changed the topic of #launchpad-reviews to: On call: jelmer || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| === mrevell is now known as mrevell-lunch | ||
| stub | https://code.launchpad.net/~stub/launchpad/pending-db-changes/+merge/43763, diff at http://paste.ubuntu.com/544036/ | 13:00 |
|---|---|---|
| stub | jelmer: ^ | 13:00 |
| jelmer | stub: Sorry, just got back from lunch | 13:25 |
| jelmer | Is the diff generator down btw? That diff still hasn't appeared. | 13:26 |
| === mrevell-lunch is now known as mrevell | ||
| jelmer | stub: Wouldn't this unnecessarily fail if the last line contained a comment? | 13:28 |
| stub | Yes, but I'd rather keep it simple rather than attempt to implement an SQL parser. | 13:29 |
| stub | I could steal the old regexp to strip comments, but that still has holes if you quote strings certain ways I think. | 13:30 |
| stub | False positives with an easily understood error message, or potential strange issues like the bug this branch is addressing. | 13:30 |
| jelmer | stub: Fair enough, though I don't think the error message is clear enough in that case. | 13:32 |
| stub | Suggestion? | 13:32 |
| jelmer | I think it should explicitly mention that the last non-whitespace character in the file should be a semicolon. if I would hit this error I'm pretty sure I would ignore comments when searching for missing semicolons. | 13:33 |
| stub | k | 13:33 |
| jelmer | stub: Other than that, r=me | 13:34 |
| stub | ta | 13:35 |
| benji | On call: jelmer, benji || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | 14:09 |
| benji | heh | 14:09 |
| === benji changed the topic of #launchpad-reviews to: On call: jelmer, benji || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jelmer | 'morning benji :-) | 14:10 |
| benji | :) | 14:10 |
| benji | leonardr: thanks for the info | 14:22 |
| leonardr | benji: i'm writing a merge proposal now. at some point i need to take a nap--i woke up at 4 am | 14:23 |
| benji | mars: I would have expected the exit code of the script to be that of the test run | 14:23 |
| benji | leonardr: k :) | 14:23 |
| benji | mars: shal I investigate? | 14:26 |
| benji | s/l/ll/ | 14:26 |
| mars | benji, please do | 14:27 |
| benji | k | 14:27 |
| mars | benji, the test log I used was (I think) test.log.1 | 14:28 |
| mars | benji, like I said in the RT, check the shell history | 14:28 |
| benji | k | 14:28 |
| mars | thanks benji | 14:29 |
| === matsubara is now known as matsubara-lunch | ||
| === allenap` is now known as allenap | ||
| === matsubara-lunch is now known as matsubara | ||
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: jelmer, benji, Edwin || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| EdwinGrubbs | hi benji, it looks like there are couple of reviews in the queue that you can do. https://code.launchpad.net/launchpad/+activereviews | 15:45 |
| benji | EdwinGrubbs: yay! I forgot to look there. I'll pick one up now. | 15:45 |
| leonardr | benji: https://code.launchpad.net/~leonardr/launchpadlib/retry-on-invalid-token/+merge/43784 | 15:56 |
| === leonardr is now known as leonardr-afk | ||
| EdwinGrubbs | jelmer: do you want to review one of my branches? The diff is actually much smaller than what the mp will say. https://code.launchpad.net/~edwin-grubbs/launchpad/bug-638924-testfix/+merge/43791 | 16:58 |
| jelmer | EdwinGrubbs: sure | 16:59 |
| jelmer | I'll make that my last review of the day though, if that's ok. | 17:00 |
| EdwinGrubbs | sure | 17:08 |
| === benji is now known as benji-lunch | ||
| jelmer | EdwinGrubbs: I need to get some dinner, I'll finish the review of your branch when I get back. | 18:56 |
| EdwinGrubbs | jelmer: ok, that's fine | 18:57 |
| jelmer | EdwinGrubbs: btw, I didn't see a linked bug for the MP but the URL suggests there is one? | 18:58 |
| EdwinGrubbs | jelmer: oops, I'll fix that | 18:58 |
| === benji-lunch is now known as benji | ||
| gary_poster | flacoste: do you happen to have any context on that revision that I'm reverting for adeuring? In a perfect world, someone with context would, since I have relatively little. The ones with context that I know of are Robert, Abel, and possibly Graham, since he reviewed Abel's branch. | 19:17 |
| gary_poster | If you don't have much/any context, I'll just throw it to the on-call reviewers and hope that we are all on the right track. | 19:17 |
| gary_poster | (The MP is https://code.edge.launchpad.net/~gary/launchpad/revert12041/+merge/43811 FWIW) | 19:18 |
| * flacoste looks | 19:18 | |
| gary_poster | The basic context is trying to turn on Robert's librarian token stuff, I think | 19:19 |
| gary_poster | I guess that didn't work out yet | 19:19 |
| flacoste | gary_poster: yep, exactly, abel wrote some code to support private attachments in bugs this summer and did it using the existing patterns of streaming the file from the private librarian using the app server | 19:20 |
| flacoste | gary_poster: this branch was supposed to remove that and serve files directly from the restricted librarian using the token-based protocol | 19:20 |
| flacoste | it fails qa | 19:20 |
| gary_poster | Got it | 19:20 |
| flacoste | your reversal looks correct, i'll approve | 19:20 |
| gary_poster | OK, thank you flacoste | 19:20 |
| flacoste | gary_poster: don't forget the magic flags --rollback when submitting | 19:21 |
| gary_poster | right, thanks :-) | 19:21 |
| EdwinGrubbs | jcsackett: I've responded to your review from last week. | 20:39 |
| * jcsackett looks. | 20:39 | |
| jcsackett | EdwinGrubbs: r=me on code*. i've requested a follow up from sinzui. thanks for those changes. | 20:41 |
| === matsubara is now known as matsubara-afk | ||
| === sinzui changed the topic of #launchpad-reviews to: On call: jelmer, benji, Edwin || Reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| sinzui | EdwinGrubbs, ping | 21:05 |
| EdwinGrubbs | sinzui: hi | 21:07 |
| sinzui | EdwinGrubbs, about your PPA MP. The message shown to the user is "Remove", but the only link I see is "Delete" | 21:09 |
| EdwinGrubbs | sinzui: good point | 21:10 |
| sinzui | EdwinGrubbs, Can I delete a PPA that has no published packages? What happens to it during the merge. It is gone or transferred | 21:10 |
| sinzui | EdwinGrubbs, actually, I know the PPA is not transferred during a merge because there is a bug about merged users having PPAs. | 21:12 |
| EdwinGrubbs | sinzui: how is that different than the bug I'm working on? | 21:13 |
| sinzui | Edwin, we should prepare a query that sets all PPAs that belong to merge users to DELETED once your branch is released | 21:13 |
| sinzui | EdwinGrubbs, because we did not recognise this bug years ago, the UI had 404s | 21:14 |
| sinzui | EdwinGrubbs, the links to the PPAs will go away when the archives are set to DELETED | 21:14 |
| EdwinGrubbs | sinzui: besides changing "Remove" to "Delete", is there anything else that needs to be changed in this branch? | 21:18 |
| sinzui | EdwinGrubbs, has_ppa_with_published_packages() will let me merge with an active ppa | 21:19 |
| sinzui | EdwinGrubbs, that ppa will still be searchable | 21:19 |
| sinzui | I think the rules must change in the method, or merge deactivates it for the user | 21:19 |
| EdwinGrubbs | sinzui: ok, I was basing this on the same restrictions as for person renames, which is what Julian suggested, however, he might have not been thinking about that situation. | 21:21 |
| sinzui | merge purges lists for users. It is okay for it to delete an empty ppa, but I think delete is slow...it goes into DELETING first | 21:22 |
| EdwinGrubbs | sinzui: maybe, I should ask someone in soyuz if that is safe to do, then. | 21:32 |
| sinzui | EdwinGrubbs, +1 | 21:34 |
| sinzui | I expect wgrant knows | 21:35 |
| === leonardr-afk is now known as leonardr | ||
| EdwinGrubbs | wgrant: ping | 21:43 |
| wgrant | sinzui, EdwinGrubbs: Just running late for a sprint... I'll be around in 20 or so. | 21:54 |
| wgrant | sinzui, EdwinGrubbs: You need the PPA to be DELETED before you do the merge. | 22:28 |
| wgrant | sinzui, EdwinGrubbs: Since it needs to remove a directory named after the user from disk. | 22:28 |
| sinzui | wgrant, thanks | 22:28 |
| EdwinGrubbs | wgrant: if the user clicks on "Delete" for the PPA, does it take place immediately, or is it queued up? | 22:29 |
| wgrant | EdwinGrubbs: Queued up. | 22:29 |
| wgrant | Since it has to happen on germanium. | 22:30 |
| wgrant | It could take half an hour or more, if things are running really slowly. | 22:30 |
| wgrant | But normally <5min. | 22:30 |
| EdwinGrubbs | wgrant: does the user get an email when it's done, so they know when they can finally merge their account? | 22:30 |
| wgrant | EdwinGrubbs: No. | 22:30 |
| thumper | anyone? https://code.edge.launchpad.net/~thumper/launchpad/bugjam-619555-request-builds-logging/+merge/43731 | 22:31 |
| EdwinGrubbs | benji: is it already your end of day? ^^^ | 22:32 |
| EdwinGrubbs | thumper: I can take it. | 22:32 |
| benji | EdwinGrubbs: nope, 30 minutes left; if it's a small one I can take it, but I'm neck deep in Leonard's review at the moment | 22:33 |
| benji | EdwinGrubbs: I can take it, it's a small one | 22:33 |
| leonardr | benji: how is the review going? do you need anything? | 22:33 |
| benji | thumper: I've got'cha | 22:34 |
| thumper | thanks | 22:34 |
| benji | leonardr: I can't think of anything, but I think it's going to have to be a two parter so I can at least get you something by EOD today; I first did a once-over to get the idea of what was going on and now I'm doing a more detailed review. I'll give you what I have in about 20 minutes. | 22:35 |
| leonardr | benji: ok, i'm almost eod as well | 22:35 |
| benji | yep | 22:35 |
| benji | you can sleep on it ;) | 22:35 |
| benji | EdwinGrubbs: I've reviewed https://code.edge.launchpad.net/~thumper/launchpad/bugjam-619555-request-builds-logging/+merge/43731 | 22:41 |
| thumper | ta | 22:41 |
| benji | thumper: since I'm being mentored, Edwin will have to approve too, of course | 22:42 |
| benji | but you're still welcome ;) | 22:43 |
| thumper | ok | 22:43 |
| benji | EdwinGrubbs: I've partially reviewed https://code.launchpad.net/~leonardr/launchpadlib/retry-on-invalid-token/+merge/43784. I don't know if you want to look at what I've done now or if you can look at the whole thing once I've finished it tomorrow. | 22:54 |
| benji | leonardr: I've put your bedtime story in a comment on https://code.launchpad.net/~leonardr/launchpadlib/retry-on-invalid-token/+merge/43784. ;) | 22:55 |
| leonardr | ok, thanks | 22:55 |
| benji | leonardr: the short version is "it looks good" | 22:55 |
| benji | leonardr: oh, one thing I forgot to mention, the tests fail for me :( | 22:55 |
| leonardr | benji: RequestTokenAuthorizationEngine has changed but i know that no one uses it right now, so i'm ok with changing it | 22:55 |
| leonardr | really. paste the errors? | 22:56 |
| benji | cool | 22:56 |
| benji | yep, pasting now | 22:56 |
| benji | leonardr: false alarm; I forgot that I did something stupid with that branch; the up-to-date verion passes just fine | 22:57 |
| leonardr | ah, ok | 22:57 |
| === benji changed the topic of #launchpad-reviews to: On call: jelmer, Edwin || Reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| EdwinGrubbs | benji: I'll try to to look at what you've done so far. | 23:01 |
| benji | thanks | 23:01 |
| benji | ok, I'm off to buy a new van (why does the material universe have to hate me?) | 23:02 |
| leonardr | benji: did your van break/crash? | 23:04 |
| benji | leonardr: yeah; it's been broken for a while, but we found out yesterday just how broken it is (transmission is not so slowly turning itself into small metal shavings) | 23:06 |
| leonardr | ah | 23:06 |
| leonardr | good luck | 23:06 |
| benji | thanks | 23:06 |
| EdwinGrubbs | thumper: is it worthwhile to create a new logger class instead of just re-using c/l/scripts/logger.py/BufferLogger | 23:23 |
| EdwinGrubbs | thumper: if there is a benfit to having it in a list instead of a StringIO, it seems like you would have a much more feature-full logger by subclassing from FakeLogger. | 23:24 |
| thumper | I didn't see any point in having a fully featured logger | 23:25 |
| thumper | I though we could add things as we need them | 23:25 |
| thumper | although looking now | 23:25 |
| thumper | perhaps bufferedlogger would have been enough | 23:25 |
| thumper | I can change | 23:26 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!