| wallyworld_ | https://code.edge.launchpad.net/~wallyworld/launchpad/link-bugs-in-merge-proposal/+merge/34826 | 00:27 |
|---|---|---|
| wallyworld_ | rockstar: ^^^^^^ you still around? Can you plz give the latest screenshot (#3, in comments) the tick of approval? | 00:28 |
| rockstar | wallyworld_, ah, yes. | 00:31 |
| rockstar | wallyworld_, looks fine to me. | 00:31 |
| wallyworld_ | rockstar: awesome, thanks | 00:32 |
| lifeless | https://code.edge.launchpad.net/~lifeless/launchpad/bug-618367/+merge/35489 | 01:12 |
| lifeless | rockstar: ^ perhaps you could eyeball that, its go a small UI caveat. | 01:12 |
| lifeless | rockstar: (A full review would be better, and only a little more effort :P) | 01:12 |
| * lifeless looks for a reviewer | 01:24 | |
| lifeless | hmm | 01:24 |
| lifeless | mwhudson: oh hai | 01:24 |
| lifeless | thumper: around? | 02:14 |
| rockstar | lifeless, hi. Still need a review? | 02:22 |
| lifeless | please | 02:23 |
| lifeless | this should squash Distribution:+assignments | 02:23 |
| lifeless | which is in https://devpad.canonical.com/~stub/ppr/lpnet/latest-daily-timeout-candidates.html | 02:25 |
| lifeless | as number 2 yesterday | 02:25 |
| rockstar | lifeless, ugh at blueprints... | 02:28 |
| lifeless | rockstar: yeah, But its holding us back. | 02:29 |
| lifeless | so, I fixinate. | 02:29 |
| rockstar | lifeless, yeah. I just hate that we have to pay the maintainer's penalty for that code at all. | 02:30 |
| lifeless | me too | 02:30 |
| lifeless | still, it could be worse. | 02:30 |
| rockstar | lifeless, one day, a happy community member may take it over. | 02:30 |
| lifeless | mtaylor will if we can just get him over the activation energy needed to get starting | 02:30 |
| lifeless | rockstar: so what do you think ? | 02:36 |
| rockstar | lifeless, can you give me a brief explanation of BatchNavigator? | 02:36 |
| lifeless | did you see my peformance tuesday mail ? | 02:36 |
| lifeless | I may have been too brief in it. So yes, I can and will explain. | 02:37 |
| lifeless | BN adapts iterables in general, ResultSets in specific for use in batches | 02:37 |
| lifeless | it is used by the API and by various web pages (e.g. branch listing, bug listings, email moderation as of yesterday, ...) | 02:37 |
| lifeless | we should probably combine it with HugeVocab's guts | 02:38 |
| lifeless | its currentBatch() function returns a slice of the iterable | 02:39 |
| lifeless | (perhaps it adapts slicable, more specificlly?..) | 02:39 |
| lifeless | the batch navigator object itself is used for generation of navigation links, popups and so on in the navigation view/templates | 02:40 |
| rockstar | lifeless, and how does this affect performance? | 02:40 |
| rockstar | If you already have the ResultSet, hasn't the query already been run? | 02:41 |
| lifeless | as long as you give it a resultset, not a listfied-resultset, its better | 02:41 |
| lifeless | rockstar: no, ResultSets are lazy | 02:41 |
| lifeless | len(rs) -> COUNT(*) | 02:41 |
| lifeless | list(rs) -> SELECT ... | 02:41 |
| lifeless | list(rs) again -> SELECT. ... (again( | 02:41 |
| rockstar | lifeless, so it somehow combines all the queries that need to run to get their resultsets? | 02:41 |
| lifeless | slice = rs[1:3] | 02:42 |
| lifeless | list(slice) -> SELECT ... OFFSET 1 LIMIT 3 | 02:42 |
| rockstar | lifeless, yeah, so how does the BatchNavigator make it faster? | 02:42 |
| lifeless | rockstar: no, the thing you adapt (in this case, self.specs) needs to be a single resultset that will Do The Right Thing | 02:42 |
| lifeless | rockstar: two ways | 02:42 |
| lifeless | firstly, by not bring back everything. | 02:42 |
| lifeless | 'specs' in the Ubuntu context has 3000 rows | 02:43 |
| rockstar | Ah, so it gives you a 500 slice. | 02:43 |
| lifeless | this patch means we'll only bring back 500 of them. | 02:43 |
| lifeless | secondly, the database can (in some circumstances) do less work when returning an LIMIT result (if the indices support that) | 02:44 |
| rockstar | lifeless, ah, okay. | 02:44 |
| rockstar | So if BatchNavigator makes a single resultset that will Do The Right Thing, what was the wrong thing? | 02:44 |
| lifeless | and thats very important in gigantic collections (like ubuntu bugs) | 02:44 |
| rockstar | Just returning 3000 results? | 02:45 |
| lifeless | but in our case its only the first thing we care about | 02:45 |
| lifeless | rockstar: BN takes a single result set and slices it. | 02:45 |
| lifeless | rockstar: the wrong thing is returning a very large slice to template code to iterate over. | 02:45 |
| rockstar | lifeless, ah! I see. | 02:45 |
| lifeless | rockstar: particularly when that may grow without bound. | 02:45 |
| lifeless | every UDS we add 500 or so specs | 02:46 |
| lifeless | maybe its only 250. Anyhow, lots. | 02:46 |
| rockstar | lifeless, we have this pattern in a few places. I wonder if it might be of interest in bring up on the the list a way to create a GenericListing page that already has some of the performance stuff built in. | 02:46 |
| rockstar | Er, we have this pattern of doing listings of many objects (sometimes without bound). | 02:47 |
| lifeless | well, BN is the generic solution | 02:47 |
| rockstar | lifeless, so would you suggest that BN be used in all listing pages? | 02:47 |
| lifeless | note that the template code is pretty close to being entirely reusable | 02:47 |
| lifeless | rockstar: we should use it whereever the folllowing is true: | 02:48 |
| rockstar | lifeless, are there any cases where BN might not be the best solution? | 02:48 |
| lifeless | - the listing can grow and grow to more than is useful for a human on the page | 02:48 |
| lifeless | - or the rows are very expensive to generate (and we've checked the sql and *everything*) | 02:48 |
| lifeless | rockstar: you'd be best to ask curtis that last one. | 02:50 |
| === StevenK changed the topic of #launchpad-reviews to: On call: StevenK || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| lifeless | I'm not sure if its ready for use on a page that shows several distinct collections | 02:51 |
| lifeless | like +activereviews | 02:51 |
| lifeless | but OTOH +activereviews is meant to be self limited | 02:51 |
| lifeless | by only showing a work queue | 02:51 |
| lifeless | rockstar: I'd certainly consider using BN if I had a listing of hundreds or thousands of things. | 02:51 |
| lifeless | rockstar: AFAIK its the only pagination solution we have | 02:52 |
| rockstar | lifeless, okay. Thanks for the run-down. | 02:52 |
| lifeless | anytime | 02:52 |
| rockstar | lifeless, why 123+ person_ids.discard(None) | 02:52 |
| lifeless | because assignee etc can be NULL | 02:53 |
| rockstar | Ah, okay. | 02:53 |
| lifeless | and we don't want to query for Person where Person.id==NULL | 02:53 |
| lifeless | phrased as Person.id in (NULL, 1, 4, 5) I suspect bad things happen :) | 02:53 |
| rockstar | lifeless, r=me | 03:00 |
| lifeless | thanks! | 03:04 |
| === jtv changed the topic of #launchpad-reviews to: On call: StevenK, jtv || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | lifeless: I believe the current procedure for db patches is to get reviews from stub and you | 03:27 |
| lifeless | jtv: https://dev.launchpad.net/PolicyAndProcess/DatabaseSchemaChangesProcess | 03:30 |
| jtv | lifeless: https://code.edge.launchpad.net/~jtv/launchpad/translationtemplatesbuild/+merge/34952 | 03:30 |
| jtv | :-) | 03:30 |
| jtv | "Say it with URLs" | 03:30 |
| lifeless | jtv: well the answer is 'no' | 03:31 |
| lifeless | but I thought more info than that would help you. | 03:31 |
| jtv | "Submit a merge proposal for your branch into lp:launchpad (lp:~launchpad-pqm/launchpad/db-devel), requesting two db reviews from both the technical architect (lifeless) and the DBA (stub)" | 03:31 |
| lifeless | yes | 03:31 |
| lifeless | keep reading | 03:31 |
| jtv | You mean the "if either is away" part? | 03:32 |
| lifeless | yes | 03:32 |
| lifeless | stub is not away | 03:32 |
| lifeless | so you need his review and the patch # will come from him | 03:32 |
| lifeless | I'm interested in the change and will look at it | 03:32 |
| lifeless | but I'm not part of the landing path | 03:32 |
| jtv | OK, then I wait for stub | 03:34 |
| jtv | lifeless: I think the wiki page implies the opposite of what you just said though. | 03:38 |
| lifeless | how so ? | 03:39 |
| jtv | The wiki page says "request reviews from both the dba and the architect; _if one of them is away_ then just the other is enough." Just now you said "stub is not away, therefore I am not necessary to the process." | 03:39 |
| lifeless | thats not quite what it says | 03:40 |
| lifeless | you always request from both | 03:40 |
| lifeless | thats what it says | 03:40 |
| lifeless | and separately | 03:40 |
| lifeless | If one of the DBA or Technical Architect is away, the other will allocate database patch numbers and provide reviews | 03:40 |
| jtv | So how is that not what I said? | 03:41 |
| jtv | It covers those two points, giving an unintentional suggestion that approval by just you _or just stub_ is a backup option for the case where the other is not available. | 03:42 |
| lifeless | thats the intent | 03:43 |
| lifeless | only one of us has to approve for it to land | 03:43 |
| lifeless | And if stub is not on leave, stub does it all. | 03:43 |
| lifeless | I'm confused | 03:43 |
| jtv | Then the page should say that. Right now it implies through omission that stub shouldn't do the review alone unless necessary. | 03:44 |
| jtv | Wait, wait, I missed a bit. | 03:44 |
| jtv | It's hidden at the end of point 4. | 03:45 |
| jtv | (I say "hidden" because when I see this much text, I instantly filter out a lot of it) | 03:47 |
| jtv | (Particularly so when the text is structured along steps-to-take lines when I want statement-of-essentials or vice versa) | 03:49 |
| lifeless | If you can improve it, do so :) | 03:50 |
| jtv | I can try | 03:50 |
| jtv | lifeless: could you have a look at the top two sections? I added the first, revised the second, deleted the third. https://dev.launchpad.net/PolicyAndProcess/DatabaseSchemaChangesProcess | 04:16 |
| jtv | I deleted the third section because it played into the confusion: when I was scanning for answers, the "help help what to do if either the dba or architect is away" header grabbed priority over the step-by-step recipe text. | 04:17 |
| jtv | Would it help if we had a launchpad-db review team and had engineers request a single db review from there? | 04:19 |
| lifeless | the diffs looked fine to me | 04:19 |
| jtv | Thanks. | 04:19 |
| * jtv dreams of a "check all review types that apply" in the MP form… | 04:20 | |
| StevenK | jtv: That's a brilliant idea | 04:20 |
| StevenK | jtv: File a bug? | 04:20 |
| jtv | StevenK: thanks—but maybe discuss it a bit further to avoid falling into the "I dreamed up this great idea that solves the problem as _I_ encounter it, I'll spec it out in great detail, and then all the engineers have to do is code it up" pattern? | 04:23 |
| jtv | _Our_ usage pattern is: we have a set of review types, each with one default reviewer, and we may need a combination of those. | 04:24 |
| jtv | Would anyone else's use differ? | 04:24 |
| jtv | I don't think so, but I'm not anyone else. :) | 04:24 |
| StevenK | jtv: No, that's not the plan. "I had this brilliant idea, let me share it with you, and we can use this usual bug report page to track it." | 04:24 |
| StevenK | s/usual/useful/ | 04:25 |
| jtv | StevenK: I'm glad that's not the plan, but I for one am human and fallible and often the victim of the very same thing. :) | 04:25 |
| jtv | Ah what the hell, you're right. | 04:26 |
| jtv | Looking through existing bugs list for a match… | 04:31 |
| jtv | StevenK, lifeless: just filed bug 638631 | 04:42 |
| StevenK | We don't have a bug bot in here? That's rather mean. | 04:42 |
| jtv | So it would seem. | 04:53 |
| jtv | stub: want db review! https://code.launchpad.net/~jtv/launchpad/translationtemplatesbuild/+merge/34952 | 06:42 |
| jtv | henninge: good morning—you're not on the other irc | 06:43 |
| henninge | jtv: Hi! ;) | 06:43 |
| * henninge reconnects the other irc | 06:43 | |
| * stub looks | 06:48 | |
| stub | TranslationTemplatesBuild instead of TanslationTemplateBuild? If we had a TranslationTemplate table it would be called TranslationTemplate | 07:06 |
| jtv | stub: well it's a build of all translation templates in a branch, and at that stage we don't even know which those will be. So there's no a-priory tie to POTemplate. | 07:11 |
| jtv | It's also consistent with the other class names; I wanted to avoid any impression that the build might belong to a single template, or to any specific template. | 07:12 |
| jtv | If, in a completely imaginary thought exercise, we renamed POTemplate to TranslationTemplate and wanted to create a reference from this new build class to TranslationTemplate, we'd have m:n and a linking table. | 07:13 |
| jtv | In that thought exercise, the linking table might be called TranslationTemplateBuild (if it weren't for the confusing similarity in names, of course) | 07:14 |
| stub | hmm... | 07:17 |
| jtv | Did I really write a-priory? I meant a-priori, of course. | 07:20 |
| stub | SPEAK ENGLISH! | 07:21 |
| jtv | oi sorry guv | 07:21 |
| jtv | A Branch goes in, a set of TranslationImportQueueEntrys comes out. Those later get assigned to respective POTemplates by the queue gardener. | 07:21 |
| stub | approved anyway | 07:23 |
| jtv | Thanks. | 07:23 |
| === StevenK changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| lifeless | https://code.edge.launchpad.net/~lifeless/launchpad/decoratedresultset/+merge/35507 | 10:28 |
| lifeless | jml: ^ care to do a micro review to start your day up ? | 10:40 |
| jml | lifeless, https://lp-oops.canonical.com/oops.py/?oopsid=OOPS-1719ED569 | 10:43 |
| lifeless | 'did not match any oops' | 10:45 |
| jml | lifeless, they take a while to be synced. | 10:48 |
| lifeless | I know | 10:48 |
| lifeless | thats on my hit list too | 10:48 |
| lifeless | but its not there yet. | 10:49 |
| lifeless | which is the worry | 10:49 |
| jml | 10 minutes, I thought. | 10:49 |
| lifeless | jml: this is the more interesting MP - https://code.edge.launchpad.net/~lifeless/launchpad/malone/+merge/35511 | 10:59 |
| lifeless | jml: and the oops still isn't visible | 11:00 |
| lifeless | jml: can you change your vote to approve, because of ec2land ? | 11:02 |
| jml | lifeless, sure. | 11:02 |
| lifeless | I really hate that style of function formatting btw | 11:03 |
| jml | lifeless, I hate the other style :) | 11:03 |
| lifeless | same as I hate it for function wrapping. | 11:03 |
| lifeless | I'm doing it, just whinging. | 11:03 |
| jml | we don't do it for calls :) | 11:03 |
| lifeless | jml: actually, its one of the 'approved styles' | 11:03 |
| * jml eyebrows | 11:05 | |
| lifeless | anyhow | 11:05 |
| lifeless | I shall blithley go on doing everything to the whims of reviewers :> | 11:06 |
| lifeless | jml: thanks for reviewing the prerequisite | 11:07 |
| lifeless | still no oops | 11:08 |
| lifeless | well, I'm off to sleep | 11:11 |
| jml | lifeless, g'night | 11:12 |
| === mrevell is now known as mrevell-lunch | ||
| === matsubara-afk is now known as matsubara | ||
| bac | hi jtv, you still reviewing? | 13:19 |
| === bac changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: - || queue: [StevenK, bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| bac | https://code.edge.launchpad.net/~bac/launchpad/bug-638420/+merge/35477 | 13:20 |
| === mrevell-lunch is now known as mrevell | ||
| jcsackett | salgado, ping. :-) | 14:23 |
| jtv | bac: sorry, bit late here… I'll update the topic. | 14:25 |
| === jtv changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [StevenK, bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| salgado | hi jcsackett, I guess your branch is ready for another look? | 14:26 |
| jcsackett | salgado: yup. sinzui had one comment on it after i pushed up changes that i've dealt with. | 14:26 |
| jcsackett | so it is ready for you to take another look at. | 14:26 |
| salgado | cool, I'll do it now | 14:32 |
| deryck | salgado, hi. Who is mentoring you for UI reviews? | 14:35 |
| salgado | deryck, sinzui | 14:37 |
| deryck | ok, thanks. | 14:37 |
| deryck | sinzui, could I get your final review of https://code.edge.launchpad.net/~deryck/launchpad/description-editing-ubuntu-font/+merge/35321 which salgado reviewed? | 14:37 |
| sinzui | I will | 14:38 |
| sinzui | deryck, salgado your have my r=me. I will update the MP to explain why will *not* be removing UbuntuBeta from any Canonical website | 14:40 |
| deryck | excellent, thanks! | 14:40 |
| bac | EdwinGrubbs: i've got a tiny branch when you start reviewing | 14:46 |
| StevenK | I suspect my branch in the queue is even tinier | 14:52 |
| salgado | jcsackett, can you give me some URLs for pages that had UI changes? | 14:52 |
| jcsackett | salgado: the one i've been using is http://launchpad.dev/thunderbird in devel vs in my branch. i can send you a screenshot of the devel version so you don't have to jump between two branches, if you like. | 14:54 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: - || queue: [StevenK, bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| salgado | jcsackett, yeah, that'd be nice, thanks | 14:55 |
| jcsackett | salgado: the changes are only there if you configure blueprints to the various not on launchpad settings (EXTERNAL, UNKNOWN or NOT_APPLICABLE) | 14:55 |
| jcsackett | ok, i'll send a screenshot in a moment. | 14:55 |
| salgado | jcsackett, for the future, in these cases it's nice to add new *dev* sampledata (e.g. projects with blueprints configured to the various possible states) so that one can easily see the changes without having to change the settings on the web UI | 14:57 |
| salgado | that helps yourself when developing and your UI reviewer. :) | 14:57 |
| jcsackett | salgado: i hadn't thought of that. i'll keep that in mind, thanks. :-) | 14:58 |
| EdwinGrubbs | StevenK: I think you meant your if-statement to be if len(sys.argv) > 1: | 14:58 |
| salgado | jcsackett, what's the difference between unknown and not_applicable? | 15:00 |
| EdwinGrubbs | StevenK: actually, you might want to allow the user to supply multiple branches on the command line, so you could set the variable as: branches = sys.argv[1:] | 15:00 |
| jcsackett | salgado: unknown is when nothing has been set: so a new project has it as the default. not_applicable represents when a project states they don't use it at all. | 15:01 |
| StevenK | EdwinGrubbs: Yes, I just came to that conclusion myself :-) | 15:01 |
| salgado | jcsackett, oh, right, I was confused because the UI seemed to be the same in both cases, but now I see that's not true | 15:02 |
| jcsackett | it is *very* similar. :-P | 15:02 |
| StevenK | EdwinGrubbs: Changes pushed | 15:03 |
| jcsackett | salgado: tar of screenshots here: http://dl.dropbox.com/u/375578/blueprints-screenshots.tar.gz | 15:18 |
| salgado | jcsackett, thanks! | 15:20 |
| jcsackett | salgado: i found one typo while making those screenshots--i'm pushing up a change that addresses it (not_applicable had the copy "Blah does not use track ..." instead of "Blah does not track ...") | 15:24 |
| salgado | jcsackett, yeah, I found that too | 15:25 |
| salgado | :) | 15:25 |
| salgado | but already sent the review, so just ignore it | 15:25 |
| jcsackett | will do. :-) | 15:27 |
| EdwinGrubbs | StevenK: since that script has a chdir() in it, relative path names don't work unless you are already in the directory it's switching to. You can probably fix that by putting the chdir() in the else-block. As long as you have an else-block, you can set branches with listdir() in there instead of before the if-statement. | 15:27 |
| === jelmer changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: - || queue: [StevenK, bac, jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | EdwinGrubbs: Can I add myself to the queue? | 16:01 |
| EdwinGrubbs | allenap: sure | 16:02 |
| === allenap changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: - || queue: [StevenK, bac, jelmer, allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | EdwinGrubbs: Thanks :) | 16:02 |
| === deryck is now known as deryck[lunch] | ||
| EdwinGrubbs | bac: r=me | 16:28 |
| === benji is now known as benji-lunch | ||
| bac | thanks edwin | 16:29 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: jelmer || queue: [StevenK, allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | Do I need to get a database review for security.cfg changes? | 16:32 |
| === benji-lunch is now known as benji | ||
| bigjools | allenap: I don't think so | 16:36 |
| allenap | bigjools: Cool, thanks. | 16:36 |
| bigjools | at least I've not bothered in the past, it affects the code more than the db | 16:37 |
| EdwinGrubbs | jelmer: the old findBuild() compared the pocket, distroseries, and archive to determine whether to raise an exception, but storeObjectsInDatabase() only compares the source_package_release. Is this right? | 16:47 |
| jelmer | EdwinGrubbs: Yes, the consistency checks from findBuild() have now moved to checkBuild() | 16:49 |
| jelmer | edwingrubbs: The idea being that checkBuild() always gets called and findBuild() will only get called in some situations (as we may pass the build object in rather than having to look for it). | 16:49 |
| EdwinGrubbs | jelmer: if the build argument is passed into storeObjectsInDatabase(), it will effectively skip the build from the first binary_package_file in bpfs_to_create. Is this intended? | 16:53 |
| jelmer | EdwinGrubbs: that will only happen if there is a single entry in bpfs_to_create (there's an assert to make sure this is the case) | 16:55 |
| === benji is now known as benji-lunch | ||
| === deryck[lunch] is now known as deryck | ||
| EdwinGrubbs | jelmer: would it make it clearer to make the block in the for-loop its own function. Right now, you are effectively passing in build as a parameter to the for-loop, which is why the logic is hard to follow. | 16:59 |
| jelmer | EdwinGrubbs: This code is about to change again for the follow-up branch, would it be ok if I improved the logic in that branch instead? | 17:01 |
| jelmer | EdwinGrubbs: I see your point though. | 17:01 |
| EdwinGrubbs | jelmer: sure | 17:01 |
| EdwinGrubbs | jelmer: r=me | 17:12 |
| jelmer | EdwinGrubbs, thanks! | 17:12 |
| === salgado is now known as salgado-lunch | ||
| === Ursinha is now known as Ursinha-lunch | ||
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: allenap || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-lunch | ||
| === benji-lunch is now known as benji | ||
| allenap | EdwinGrubbs: Are you reviewing bugsubscription-to-storm or my other branch? I've renamed the other one so any browser windows you have open to the merge proposal will fail form submission. It's now https://code.edge.launchpad.net/~allenap/launchpad/bug-subscription-filter-models-bug-639749/+merge/35546 | 18:02 |
| allenap | EdwinGrubbs: Also, I have to go now; dinner time with the kids. Is that okay? | 18:03 |
| EdwinGrubbs | allenap: which one do you want me to review? It's fine if you need to leave. | 18:07 |
| === Ursinha-lunch is now known as Ursinha | ||
| === salgado-lunch is now known as salgado | ||
| jelmer | EdwinGrubbs: Would you have time for another review ? It's the follow-up to the other branch that you reviewed. | 18:45 |
| === matsubara-lunch is now known as matsubara | ||
| EdwinGrubbs | jelmer: sure | 18:46 |
| jelmer | EdwinGrubbs, The MP is at https://code.edge.launchpad.net/~jelmer/launchpad/506256-remove-popen-2/+merge/35412 | 18:50 |
| jelmer | EdwinGrubbs: Unfortunately it contains the diff for the other branch as well, since it hasn't landed yet and I already have a different prerequisite branch | 18:51 |
| EdwinGrubbs | jelmer: wow, I merged in both archiveuploader-build-handling nad 506256-remove-popen, and 506256-remove-popen-2 is still 1300 lines. | 18:58 |
| EdwinGrubbs | jelmer: are there any other prereq branches besides those two? | 18:59 |
| lifeless | EdwinGrubbs: https://code.edge.launchpad.net/~lifeless/launchpad/malone/+merge/35511 | 19:01 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: jelmer || queue: [allenap, lifeless] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jelmer | EdwinGrubbs: No, there aren't any others. If it's 1300 lines even with those branches merged in, perhaps I should have a look at splititng it up further. | 19:02 |
| jelmer | EdwinGrubbs: I'll mark it as on hold for the time being - sorry for the trouble. | 19:05 |
| EdwinGrubbs | no problem | 19:05 |
| lifeless | EdwinGrubbs: hi? | 19:43 |
| EdwinGrubbs | lifeless: hi | 19:44 |
| lifeless | are you reviewing allenaps still ? or I can has review? | 19:45 |
| EdwinGrubbs | lifeless: I'll be done with his shortly | 19:46 |
| lifeless | cool | 19:46 |
| lifeless | I'm just excited :) | 19:46 |
| EdwinGrubbs | it's my job as a reviewer to crush excitment | 19:47 |
| lifeless | nae knave, tis not | 19:48 |
| lifeless | EdwinGrubbs: btw - bugsubscription-to-storm - we need a storm base class with cache integration before moving things with cachedproperty around (maybe I'm wrong and it doesn't have any) | 20:02 |
| EdwinGrubbs | lifeless: ok, I'm probably only going to review allenap's other branch today. | 20:04 |
| lifeless | EdwinGrubbs: do you mean I should another reviewer? (Just occured to me there are multipe parse trees for your statement) | 20:11 |
| EdwinGrubbs | lifeless: oh, no. I'll review your branch today, also. | 20:11 |
| lifeless | cool | 20:13 |
| jcsackett | EdwinGrubbs: i've got an MP to throw in the queue, are you likely to have time for it? | 20:59 |
| lifeless | jcsackett: I'm on call now too | 21:00 |
| lifeless | submarine style | 21:00 |
| lifeless | jcsackett: whats the MP | 21:00 |
| jcsackett | lifeless: https://code.edge.launchpad.net/~jcsackett/launchpad/user-email-existing-account-576757/+merge/35575 | 21:00 |
| jcsackett | it's fairly simple. | 21:00 |
| EdwinGrubbs | jcsackett: in about 1.5 hours | 21:00 |
| jcsackett | EdwinGrubbs: unless i'm mistaken, lifeless is offering to take mine. | 21:01 |
| EdwinGrubbs | oh, nevermind | 21:02 |
| jelmer | EdwinGrubbs: I've got an updated (smaller) branch when you're back. | 21:03 |
| lifeless | jcsackett: what is it | 21:03 |
| lifeless | sorry | 21:03 |
| lifeless | jelmer: what is it | 21:03 |
| lifeless | jcsackett: done | 21:03 |
| lifeless | jelmer: whats the MP url | 21:06 |
| jelmer | lifeless: https://code.edge.launchpad.net/~jelmer/launchpad/no-more-buildid/+merge/35572 | 21:06 |
| jelmer | lifeless: removing the --buildid argument from archiveuploader and access to the command line options object from deep inside archiveuploader | 21:06 |
| jcsackett | lifeless, thanks. | 21:08 |
| lifeless | jelmer: done | 21:09 |
| jelmer | lifeless: Thanks! | 21:09 |
| lifeless | EdwinGrubbs: I'm pushing up rev 11549 to my branch, just some fine tuning | 21:18 |
| EdwinGrubbs | lifeless: review sent. The only comment I have about rev 11549 is the same thing I noted already about store.using(tables) with storm objects. | 21:35 |
| lifeless | thanks | 21:38 |
| lifeless | EdwinGrubbs: I might stay with the literal string here | 21:41 |
| EdwinGrubbs | lifeless: If you stick with string literals, could you format them so they are easier to read? For example, one table per line, and if the conditional is really long, indent it over multiple lines. | 21:55 |
| lifeless | I'll fiddle with it a little yes | 21:56 |
| === matsubara is now known as matsubara-afk | ||
| === Ursinha is now known as Ursinha-afk | ||
| lifeless | I need a review https://code.edge.launchpad.net/~lifeless/launchpad/lessGetLastOops/+merge/35605 | 23:17 |
| lifeless | this may help with the ec2 crashes | 23:17 |
| * jelmer looks | 23:18 | |
| jelmer | lifeless: r=me | 23:23 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!