| gary_poster | I need a testfix review | 00:27 |
|---|---|---|
| gary_poster | http://pastebin.ubuntu.com/410297/ | 00:27 |
| gary_poster | The problem was that buildbot is the only thing that runs ``python2.5 -t test_on_merge.py -vv`` | 00:28 |
| gary_poster | ec2 test does it differently (not sure how) so it was missed. | 00:28 |
| gary_poster | I'd prefer to change buildbot to simply run ``test_on_merge.py -vv`` at which point I can remove the nasty os.execv | 00:29 |
| gary_poster | but this lets us move on for now | 00:29 |
| gary_poster | mwhudson: ping? | 00:30 |
| mwhudson | gary_poster: looking | 00:30 |
| mwhudson | gary_poster: that's horrible | 00:31 |
| gary_poster | yes it is | 00:31 |
| mwhudson | gary_poster: changing buildbot isn't that hard, really | 00:31 |
| gary_poster | ...no, but I should be giving my son a bath :-) | 00:32 |
| mwhudson | at least if there's a losa around | 00:32 |
| gary_poster | I suppose I could just back my branch out | 00:32 |
| mwhudson | gary_poster: how should buildbot invoke test_on_merge.py ? | 00:33 |
| mwhudson | gary_poster: just with ./bin/py ? | 00:33 |
| gary_poster | mwhudson: that would work. Invoking it with ``test_on_merge.py -vv`` would also work. Then the diff woud be this: | 00:34 |
| mwhudson | gary_poster: like so https://pastebin.canonical.com/30183/ ? | 00:34 |
| gary_poster | mwhudson: exackle. | 00:35 |
| mwhudson | gary_poster: i can get that change landed if you like | 00:36 |
| mwhudson | it seems better than that pastebin, tbh | 00:36 |
| gary_poster | mwhudson: testfix would then be http://pastebin.ubuntu.com/410305/ | 00:37 |
| mwhudson | gary_poster: ah right | 00:38 |
| gary_poster | mwhudson: I'll see if a losa is around. If so, will do it. If not, will take you up on your offer with gratitude | 00:38 |
| mwhudson | gary_poster: want to review https://code.edge.launchpad.net/~mwhudson/lpbuildbot/bin-py-test/+merge/22918 ? | 00:39 |
| mwhudson | it's probably a good idea in any case | 00:39 |
| gary_poster | reviewing | 00:39 |
| * mwhudson wonders if anyone wants to review https://code.edge.launchpad.net/~mwhudson/launchpad/allow-import-password/+merge/22924 | 06:32 | |
| jtv | mwhudson: I'm on it | 08:12 |
| === StevenK changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | mwhudson: there's no chance that these passwordful URLs will show up in the UI? | 08:14 |
| mwhudson | jtv: there's every chance | 08:14 |
| jtv | mwhudson: isn't that, like, potentially bad? | 08:14 |
| mwhudson | jtv: not really, it's for cases that require guest:guest as the username/password combo | 08:14 |
| mwhudson | jtv: i guess some words in the new code import form might be a good idea | 08:15 |
| jtv | mwhudson: the blink tag almost seems like a good idea here, if you get my point... | 08:15 |
| * wgrant banishes jtv. | 08:16 | |
| jtv | hey, I said "almost"! | 08:16 |
| mwhudson | jtv: in general, the result of the import is going to be publicly available | 08:16 |
| jtv | mwhudson: that's fine, but one thing that gives a black-hat an even better guess to all of the average user's passwords than their birthday is... one of their passwords. | 08:17 |
| jtv | Not that bzr users are likely to be average computer users. But OTOH they're likely to manage more accounts than most. | 08:18 |
| jtv | Another way to do it would be to obscure the password when displaying the URL. | 08:19 |
| jtv | mwhudson: Actually I'd prefer the latter because it also protects people who enter a login that has commit rights. Whether those people deserve protection is a matter for BOFHs and PFYs, I guess. :-) | 08:21 |
| jtv | mwhudson: I have to relocate now. I've added my comment to the MP. | 08:31 |
| === jtv changed the topic of #launchpad-reviews to: On call: jtv || reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | On call: jtv || reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | 10:16 |
| StevenK | jtv: Hi! | 10:17 |
| jtv | hi StevenK | 10:17 |
| jtv | Your code looks good, but the MP is a bit thin! Pre-imp? Lint? | 10:17 |
| StevenK | jtv: Uh? I'm new here, can you expand 'pre-imp' ? | 10:20 |
| jtv | Ah. "Pre-implementation call." | 10:21 |
| jtv | We're expected to discuss each change with someone—whether it's someone more knowledgeable about the topic, or a peer, or an outsider, or someone less experienced—and make every effort to make that discussion by voice. | 10:22 |
| * wgrant imagines the content of the call as "AAAAAAAAAAA it's broken it's broken fix it fix it" | 10:22 | |
| jtv | wgrant: the intended solution should come up at least in passing, thank you. | 10:22 |
| StevenK | jtv: Oh, right. In both MPs I've got up for review, I've spoken to both Julian Edwards and Michael Nelson via Skype about them | 10:22 |
| jtv | But otherwise, wgrant hit it on the nose. :) | 10:23 |
| jtv | Good! | 10:23 |
| jtv | Because as a non-Soyuz person I'm hardly in a position to know whether this change is as sane as it looks. | 10:23 |
| jtv | And with "lint," I mean the output of "make lint." I'm not assuming you didn't run it, but we normally at least mention it in the MP as a reminder. | 10:24 |
| jtv | My own branches generally have a final commit around MP time with the description "Lint." Shows how effective that procedure can be. | 10:24 |
| StevenK | I've been reading through the coding standards, but I have yet to get into the habit of running make lint before submitting the MP | 10:25 |
| jtv | Then this would be a good start! | 10:25 |
| jtv | Good *time to* start, that is | 10:25 |
| StevenK | Well, running it just blew up with a wierd error, so \o/ | 10:25 |
| jtv | Unfortunately a lot of our standards aren't properly documented afaik | 10:25 |
| jtv | uh-oh | 10:26 |
| StevenK | OSError: [Errno 17] File exists: '/home/steven/launchpad/lp-branches/fixes-bug-387049/eggs/setuptools-0.6c9-py2.5.egg' | 10:26 |
| jtv | Always fun how EEXISTS never mentions why this should be a problem. | 10:27 |
| jtv | You could try removing it, but then you'd probably also have to re-run rocketfuel-get; make clean; utilities/link-external-sourcecode ../devel; make | 10:28 |
| jtv | (And possibly merge devel somewhere in that sequence as well) | 10:28 |
| * jtv tries the branch out locally | 10:28 | |
| StevenK | Well, I'm happy to make clean and then link and make lint | 10:29 |
| jtv | I can do it here—you ran the test though, right? | 10:30 |
| StevenK | The test suite? Yes | 10:31 |
| jtv | So it's not really worth going through the pain just for the lint. Hang on, I'm setting things up on my end. | 10:31 |
| jtv | About the code itself: nice to know about the test publisher's addMockFile and addFakeChroots; I wasn't aware of those. | 10:32 |
| jtv | StevenK: ah, this should be helpful: https://dev.launchpad.net/CoverLetters | 10:33 |
| === daniloff is now known as danilos | ||
| jtv | StevenK: meanwhile, I have a call coming up. That means I'll drop off for a while, without much warning. | 10:33 |
| StevenK | Right, I have make lint working and running at least | 10:34 |
| jtv | oh good | 10:34 |
| === jtv is now known as jtv-otp | ||
| StevenK | But understanding the output is the next problem | 10:36 |
| === allenap changed the topic of #launchpad-reviews to: On call: jtv, allenap || reviewing: -, - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | Morning jtv-otp :) | 10:45 |
| jtv-otp | hallo allenap | 10:45 |
| === jtv-otp is now known as jtv | ||
| jtv | StevenK: the core of the change could be made a tad easier to read, I think (always a concern with these long soyuz identifiers). | 10:49 |
| jtv | StevenK: something like | 10:50 |
| jtv | dest_file = dfc.get(file.libraryfile.filename) | 10:50 |
| jtv | if dest_file is not None and file.libraryfile != dest_file: | 10:50 |
| jtv | (Saves an indent level as well) | 10:50 |
| jtv | StevenK: _but_ "file" is not a good variable name since it's also a built-in class. This has bitten me many times, though not usually very hard. | 10:51 |
| StevenK | Yes, it has me too. I'm not sure why I keep doing 'for file in ...' | 10:51 |
| jtv | If it bit harder, I'm sure we'd all have learned by now. :-) | 10:52 |
| StevenK | jtv: So I've got the output from make lint, with stuff like: 652: [E1002, CopyCheckerTestCase.setUp] Use super on an old style class | 10:52 |
| jtv | StevenK: that particular message is... "a bit confused" afaict. You have my wholehearted fiat to ignore that one until we know what python version we're really at. | 10:53 |
| StevenK | Hehe, right | 10:54 |
| StevenK | jtv: So, I'll clean up that code tomorrow and push a branch | 10:54 |
| jtv | My compliments on your commenting habits: it's no more and no less than what's needed. The one at the top of the diff though is part of a "# Do X" pattern. It would probably be nicer to have the new check in a separate method, and its name could do all the explaining needed. | 10:55 |
| StevenK | jtv: I thought about doing that -- but the method is already doing lots of checks, and it was either adding one more, or a refactor | 10:56 |
| jtv | Well one has to start somewhere... I'm not particularly keen on fixing this pattern, usually, but when you build up real data structures, especially in longer methods, it's often very helpful to minimize their scope. | 10:57 |
| jtv | I do agree that it's ugly to have different levels of abstraction in the method; maybe it's something that fits better one level up in the call tree? | 10:57 |
| StevenK | jtv: Thinking about it more, this method already has a large cache structure that is used by a large number of the checks, and passing that around just makes me feel even uneasier | 10:58 |
| jtv | StevenK: is this an urgent fix, something you may want to CP? | 10:59 |
| StevenK | jtv: It isn't, no | 10:59 |
| StevenK | jtv: It's a nice to have, since it may bite people who use the package copier | 10:59 |
| jtv | Because when you hit this sort of dilemma, it's probably best to "bzr shelve" your changes (or create a pipeline) and put up a separate refactoring branch first. Of course by the time one realizes this, it's often water under the bridge. | 11:00 |
| StevenK | Heh, yes | 11:01 |
| jtv | Looking at the diff, ISTM you only need to pass a few things if you extract this (into a standalone function afaic): destination_file_conflicts and source.sourcepackagerelease.files. | 11:04 |
| StevenK | jtv: Right, what I was getting at, was if I abstract this bit out, why not the rest too? | 11:04 |
| StevenK | Where my concern about copying around large structures came from | 11:05 |
| jtv | Well, that would be good too, definitely! I thought you were concerned that I might force you to, and was trying to assuage that concern. | 11:05 |
| === jtv changed the topic of #launchpad-reviews to: On call: jtv, allenap || reviewing: StevenK, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| StevenK | Wah, now bin/test is blowing up | 11:14 |
| wgrant | I have a severely oversized (~9500 lines) branch, but it is exclusively removals. Can I can convince somebody to take it? | 11:14 |
| jtv | wgrant: imagine a very noncommittal voice here... removals of what? | 11:15 |
| wgrant | jtv: Lots of unused code. A JS library or two, an extraneous JS minifier, some directories named 'not-used', a couple of Makefiles that have been unused and broken for three or four years... | 11:16 |
| StevenK | jtv: So I have the refactoring changes, but bin/test is blowing up with ImportError: No module named gettextpo | 11:16 |
| jtv | wgrant: wow... for the JS stuff at least, I'd ask sinzui; he probably has the best overview of what can go. | 11:17 |
| jtv | StevenK: you may need to "make" again | 11:17 |
| StevenK | Ahh | 11:19 |
| jtv | StevenK: the only lint I see btw is test_copypackage.py importing TestCase but not using it. | 11:19 |
| StevenK | jtv: Right, which I've fixed locally | 11:22 |
| StevenK | Whee, Failed to load application: No module named psycopg2 | 11:24 |
| wgrant | StevenK: Your version of python-support or python-psycopg2 is wrong. | 11:25 |
| wgrant | Ah, no, just the latter. | 11:25 |
| StevenK | So karmic's version of python-psycopg2 is wrong? | 11:25 |
| wgrant | You need one from the Launchpad PPA. | 11:27 |
| StevenK | Which is installed | 11:28 |
| StevenK | *** 2.0.8-0ubuntu3~launchpad1 0 | 11:28 |
| wgrant | What happens if you try to import psycopg2 from a non-buildout-tainted python2.5? | 11:29 |
| StevenK | wgrant: It works | 11:31 |
| wgrant | <3 buildout <3 | 11:34 |
| StevenK | Hah, nice. :-( | 11:42 |
| allenap | StevenK: You have two proposals needing review. Shall I take one? | 11:48 |
| StevenK | allenap: fixes-bug-553068 is the other, so sure | 11:50 |
| allenap | StevenK: Cool. | 11:53 |
| === allenap changed the topic of #launchpad-reviews to: On call: jtv, allenap || reviewing: StevenK, StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: jtv, allenap || reviewing: StevenK, StevenK || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | Hi jtv, allenap. This one's already been reviewed, but I had to create a new MP after failing to merge a new production-devel: https://code.edge.launchpad.net/~michael.nelson/launchpad/pd-550049-overwriting-buildd-manager-files/+merge/22932 | 11:58 |
| jtv | noodles775: yowza... "you don't have permission to access this page" | 11:59 |
| * noodles775 adds jtv | 11:59 | |
| noodles775 | jtv: try now. | 12:00 |
| jtv | yup, that works. | 12:00 |
| noodles775 | Thanks. | 12:01 |
| jtv | noodles775: two very different ideas on where buildid should come from, there... | 12:01 |
| jtv | Used to come from the slave, now comes from the master. | 12:01 |
| jtv | What method is that in? I'm talking about lines 17—18 of the diff. | 12:02 |
| noodles775 | jtv: from the build itself. If you look at the original MP, there's a note there about it. | 12:03 |
| noodles775 | jtv: and it's _handleStatus_OK | 12:03 |
| * wgrant points at the channel privacy and coughs a bit. | 12:03 | |
| jtv | wgrant: thank you, it had percolated through in the meantime | 12:04 |
| * noodles775 moves the discussion elsewhere :) | 12:07 | |
| wgrant | Thanks. | 12:07 |
| === mrevell is now known as mrevell-lunch | ||
| === matsubara-afk is now known as matsubara | ||
| jtv | StevenK: any progress on the 387049 branch? | 13:09 |
| StevenK | jtv: You know it's 10pm here, right? :-) | 13:10 |
| jtv | StevenK: no, but thanks for telling me. :) | 13:10 |
| === jtv changed the topic of #launchpad-reviews to: On call: allenap || reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | eod here too... | 13:11 |
| === mrevell-lunch is now known as mrevell | ||
| gmb | allenap, Could you review the code portion of https://code.edge.launchpad.net/~gmb/launchpad/not-nullify-bwa.result-bug-546774/+merge/22934 for me? | 13:56 |
| gmb | stub, BjornT, I've requested DB reviews from you guys for https://code.edge.launchpad.net/~gmb/launchpad/not-nullify-bwa.result-bug-546774/+merge/22934, too. | 13:57 |
| gmb | stub, Thanks for the review :) | 14:03 |
| allenap | gmb: Sure. | 14:16 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: gmb || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | allenap: hadn't even noticed you were reviewing my branch... thanks! | 14:30 |
| allenap | jtv: Cool, you're welcome :) | 14:31 |
| jtv | :) | 14:32 |
| jtv | and with that, jtv eods | 14:32 |
| jtv | good night! | 14:32 |
| === sinzui changed the topic of #launchpad-reviews to: On call: allenap || reviewing: gmb || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | allenap, EdwinGrubbs: can you review https://code.launchpad.net/~sinzui/launchpad/newline-removal/+merge/22915 | 14:55 |
| EdwinGrubbs | sinzui: sure | 14:55 |
| gary_poster | kfogel: re https://code.edge.launchpad.net/~kfogel/launchpad/google-analytics-everywhere/+merge/22891 , it's fine. I would remove the lines with only ellipses, because I don't think they buy anything, but if you disagree, I'm ok with it as is. | 14:55 |
| kfogel | gary_poster: that's a good idea; I wasn't sure quite how "..." matching worked and actually meant to test without those lines, then forgot to. | 14:58 |
| gary_poster | kfogel: it's very greedy, think ``.*`` | 14:58 |
| kfogel | gary_poster: ok | 14:59 |
| gary_poster | kfogel: fwiw actually, I think ``.+`` is more accurate | 14:59 |
| sinzui | BjornT: can you look at the proposed schema/interfaces changes in https://code.edge.launchpad.net/~sinzui/launchpad/not-packaged/+merge/22810 | 14:59 |
| kfogel | gary_poster: :-) gotcha | 14:59 |
| BjornT | sinzui: sure | 15:01 |
| gary_poster | kfogel: approved | 15:03 |
| kfogel | gary_poster: thanks. (I committed the change you suggested; test still passes) | 15:04 |
| gary_poster | :-) cool | 15:04 |
| BjornT | sinzui: i have some issues with the name. also, only using a date seems a bit unclear. did you consider other options? | 15:11 |
| sinzui | BjornT, We considered recording the series, but that leads to asking redundant questions... | 15:12 |
| sinzui | The new series is always identical to the series just released...not enough time has passed for the series to diverge... | 15:13 |
| sinzui | So we think 1 year is about the right time for users. the real time is closers to 9 months | 15:13 |
| sinzui | BjornT, If someone can suggest a better name for the date field I will happily change it. | 15:15 |
| * sinzui struggled with the name | 15:15 | |
| BjornT | sinzui: did you consider using the existing Packaging table for this? | 15:16 |
| sinzui | BjornT, only briefly. The schema requires a distroseries. I think it would get message to create a entry without a distroseries | 15:19 |
| sinzui | BjornT, and it requires a SPN, which the project does not have. | 15:20 |
| sinzui | s/message/messy/ | 15:21 |
| BjornT | sinzui: well, the db schema doesn't require it, as far as i can see, so it's easily changable. | 15:21 |
| sinzui | what? | 15:22 |
| sinzui | The disrtoseries+spn is a unique key I think | 15:22 |
| BjornT | sinzui: they can still both be null. | 15:25 |
| sinzui | BjornT, you propose that we create a false packaging link for the project. And we modify product+packages to hide the false packaging link. | 15:26 |
| BjornT | sinzui: i want you to consider using the Packaging table, since it gives you the date and who said it wasn't packaged for free. | 15:26 |
| sinzui | right | 15:27 |
| BjornT | sinzui: i'm not proposing a false link. i propose to say that it isn't linked to a source package | 15:27 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | We do not really delete links. So if I have a false link, and then visit +ubuntupkg, do I need to delete the false link when I create the new one | 15:28 |
| sinzui | BjornT, I suppose so, and I need to delete it for sp+linkpackage | 15:29 |
| BjornT | sinzui: yes, or update it. we do allow links to be deleted, now? | 15:29 |
| BjornT | sinzui: in short, you should do the same as you do when links are edited. | 15:29 |
| sinzui | We cannot update it in the latter case, Remember that from the sp's perspective there is a 1-1 relationship and it queried for iteself and found None. projectseries know the relationship is *-* | 15:30 |
| sinzui | BjornT, yes, we allow users to delete links. the application creates them, or updates the distroseries-spn version. This is a minor detail. I'll give the Packaging approach a try | 15:33 |
| BjornT | sinzui: well, you can always retrieve the None-link, if you have no link. it's the same things as if you want to link a package to a project that already has a link to another package, except you don't have to ask the user what the right link should be. | 15:34 |
| BjornT | sinzui: cool. whether to use the distroseries is an open question. setting it to the current series might be good, but i'm not sure. i suspect it's more correct to set it, since Packaging (at least in theory) is used for more than Ubuntu. (or isn't it allowed to link to non-Ubuntu packages?) | 15:36 |
| sinzui | We removed the feature to allow projects to link to other distros. Other distros can link to a project, but given the fact we only have spph for Debian, only Debian can get a link | 15:38 |
| sinzui | distroseries + spn is unique. I really really do not want to change that rule. This rule as added to prevent the insane data we collected for the first 3 years of the feature | 15:39 |
| sinzui | BjornT, As I think about the distroseries+spn constraint, I do not think Packaging can be used. We need to manufacture an spn for each project | 15:43 |
| BjornT | sinzui: there shouldn't be a need to manufacture an spn. the current constraints allow spn to be null. | 15:44 |
| BjornT | sinzui: you might have to to add a constraint to make sure there's only one null spn for a project/distroseries, though | 15:45 |
| sinzui | I do not think we can have (null, null, gdp/trunk) and (null, null, launchpad/trunk) | 15:46 |
| BjornT | sinzui: you can with the current constraints | 15:47 |
| sinzui | Bjorn: None isn't acceptable as a value for Packaging.distroseries | 15:54 |
| sinzui | BjornT, storm does not like it | 15:55 |
| sinzui | BjornT, We need to change the model to allow spn and distroseries to allow None | 15:57 |
| BjornT | sinzui: well, that's because storm has been told it can't None, even though the db schema allows it | 15:57 |
| BjornT | sinzui: yes, you need to update the model class and interface for this to work. you probably won't come up with a solution that doesn't require any model changes :) | 15:58 |
| sinzui | Deleting a product series and the latest packages get complicated. We will need to revise or add methods that get SPs to know that they can get meaningless results that need filtering | 15:59 |
| BjornT | sinzui: i don't see what would be complicated. could you expand? | 16:00 |
| sinzui | Product.sourcepackages must change to ensure false packaging links are not uses...conversely, this is the method we use when choosing to ask the user to link to package. We need another method link sourcepackages that does not filter | 16:02 |
| === danilos is now known as daniloff | ||
| BjornT | sinzui: right. i wouldn't call it complicated, though, just normal code changes, which you generally have to deal with :) | 16:04 |
| sinzui | BjornT, The same is true for productseries. And setPackaging can be taught to work with false packages. | 16:04 |
| * BjornT -> phone | 16:07 | |
| sinzui | BjornT, okay, I will try. this will be much more invasive, requiring many more code changes to disambiguate the Packaging object. | 16:08 |
| leonardr | allenap, for the queue: https://code.edge.launchpad.net/~leonardr/lazr.restful/cache-service-root/+merge/22948 | 16:20 |
| allenap | leonardr: I'll do it now. | 16:20 |
| leonardr | great | 16:20 |
| allenap | leonardr: Oops, I didn't see sinzui's entry. sinzui, is that the newline-removal branch? | 16:22 |
| sinzui | allenap, yes it is | 16:22 |
| allenap | sinzui: Okay. leonardr, I'll do yours after sinzui. | 16:22 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: sinzui || queue: [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === deryck is now known as deryck[lunch] | ||
| === salgado is now known as salgado-lunch | ||
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === gary_poster is now known as gary-lunch | ||
| allenap | leonardr: Is there any way config.active_versions could be empty? | 17:00 |
| leonardr | allenap: no, that would prevent lazr.restful from starting up | 17:00 |
| allenap | leonardr: Instead of having [604800, 3600], could you have a module-level constant like `HOUR = 3600 # seconds`, then the default can be specified as [7 * 24 * HOUR, 1 * HOUR]? | 17:02 |
| leonardr | sure | 17:03 |
| EdwinGrubbs | allenap: oops, I forgot to take sinzui's name out of the queue and claim the review. I didn't mean to re-review it. | 17:05 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: allenap, Edwin || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | I have 3 reviews? I must be special | 17:05 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap, Edwin || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | EdwinGrubbs: All's well; you spotted a couple of things I missed. | 17:06 |
| leonardr | allenap, i had to make one more minor change to https://code.edge.launchpad.net/~leonardr/lazr.restful/cache-service-root/+merge/22948 | 17:18 |
| allenap | leonardr: I'll take a look. | 17:18 |
| allenap | leonardr: Looks good. | 17:20 |
| allenap | EdwinGrubbs: If you have a moment, please could you sanity check the diff I've attached to https://code.edge.launchpad.net/~allenap/launchpad/dynamic-batch-size-bug-546085/+merge/22538 | 17:33 |
| allenap | EdwinGrubbs: I added a constant to signify an unlimited size batch, BATCH_SIZE_UNLIMITED, but it's 0, not object(). However, the meaning when batch_size is None is now unambiguous: it always means unset, never unlimited. | 17:35 |
| leonardr | allenap, looks like i need to do still more work on the lazr.restful side, i'll ping you when it's ready | 17:35 |
| allenap | leonardr: Okay, no worries. I'll be afk in ~20 minutes, but I'll look at it in the morning. | 17:37 |
| leonardr | allenap: i may have someone else look at the updates | 17:37 |
| leonardr | in that case | 17:37 |
| allenap | leonardr: Okay. | 17:37 |
| EdwinGrubbs | allenap: I'm surprised that you added a line to tests/externalbugtracker.py to set the batch_size to BATCH_SIZE_UNLIMITED, but it didn't change the results of any of the tests. | 17:41 |
| allenap | EdwinGrubbs: It kind of does the same as lines 153 to 157 used to do. | 17:44 |
| allenap | EdwinGrubbs: ... because most calls into updateBugWatches() and _getRemoteIdsToCheck() didn't specify batch_size. | 17:44 |
| EdwinGrubbs | allenap: ok, that makes sense. Looks good. | 17:45 |
| allenap | EdwinGrubbs: Thanks :) | 17:46 |
| === deryck[lunch] is now known as deryck | ||
| === salgado-lunch is now known as salgado | ||
| === matsubara is now known as matsubara-lunch | ||
| === gary-lunch is now known as gary_poster | ||
| === matsubara-lunch is now known as matsubara | ||
| === leonardr is now known as leonardr-lunch | ||
| === leonardr-lunch is now known as leonardr | ||
| EdwinGrubbs | mars: I see that you have two merge proposals for the same branch to merge it into devel and db-devel. Can one of those be deleted? | 21:30 |
| === salgado is now known as salgado-afk | ||
| === matsubara is now known as matsubara-afk | ||
| === mwhudson_ is now known as mwhudson | ||
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: - || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!