| === Ursinha-afk is now known as Ursinha | ||
| sinzui | Edwin: I approved your branch | 03:17 |
|---|---|---|
| EdwinGrubbs | benji: r=me | 03:44 |
| EdwinGrubbs | sinzui: thanks | 03:44 |
| === StevenK changed the topic of #launchpad-reviews to: On call: Edwin || Reviewing: - || queue: [benji,StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === Ursinha is now known as Ursinha-afk | ||
| EdwinGrubbs | StevenK: are you a reviewer? | 05:14 |
| StevenK | EdwinGrubbs: I am, but my mentor is on holidays | 05:17 |
| * mwhudson can mentor something if it's easy | 05:17 | |
| mwhudson | (not because i'm lazy but because i need to disappear fairly soon) | 05:18 |
| EdwinGrubbs | StevenK: well, I want you to look at the branch since it is a soyuz testfix for db-devel. | 05:19 |
| StevenK | EdwinGrubbs: Sure, point me to the MP? | 05:20 |
| EdwinGrubbs | StevenK: here it is: https://code.edge.launchpad.net/~edwin-grubbs/launchpad/broken_test_do_not_copy_disabled_dases/+merge/37815 | 05:24 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [benji,StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| StevenK | Oh, that's my fault :-/ | 05:24 |
| === EdwinGrubbs 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 | ||
| StevenK | EdwinGrubbs: The code + test that was on db-devel pre-dates the devel code, and I was meaning to fix it, but sleep got in the way. | 05:25 |
| StevenK | EdwinGrubbs: So, I can tell you my ideas on the MP, or I can just fix it since it's my fault. Which do you think? | 05:29 |
| EdwinGrubbs | StevenK: you can just fix it. | 05:30 |
| StevenK | EdwinGrubbs: Okay, doing so, thanks. | 05:32 |
| wgrant | StevenK: Quick Soyuz one, if you're interested: https://code.edge.launchpad.net/~wgrant/launchpad/bug-655648-a-f-maverick/+merge/37820 | 06:53 |
| StevenK | wgrant: Added my code* to it | 06:55 |
| wgrant | StevenK: Thanks. | 06:57 |
| noodles775 | Hi stub, would you have time for a tiny db-review? https://code.launchpad.net/~michael.nelson/launchpad/638090-base_version-property-for-differences/+merge/37742 | 07:47 |
| * stub has a look | 07:59 | |
| stub | noodles775: Might we need to join or lookup rows using base_version ? I'm wondering if it needs an index or not. | 08:06 |
| noodles775 | stub: Hrm... I can't think of an example. Currently the use-case is, here's a (loaded) difference, do you want to request a PackageDiff to be generated between the base version and the derived version etc. But right, it might save pain in the future when we add a query and assume an index? | 08:10 |
| stub | noodles775: Sounds like we don't need it. | 08:21 |
| stub | noodles775: r=stub patch-2208-24-0.sql | 08:23 |
| noodles775 | Great, thanks stub! | 08:23 |
| bac | hi henninge | 09:28 |
| henninge | Hi bac! | 09:35 |
| * henninge looks at activereviews | 09:36 | |
| bac | hi henninge, could you have another look at my MP? sinzui did the UI mentor | 09:36 |
| bac | henninge: i'm also having some trouble with a translation test and need some domain-specific geniusity | 09:37 |
| henninge | oh, is there a planned downtime for code hosting atm? | 09:40 |
| henninge | bac: I imaging the help link looks just like what it looks on the project page, right? | 09:41 |
| henninge | codehosting is back | 09:41 |
| bac | yes indeedy | 09:41 |
| henninge | bac: r=me | 09:43 |
| henninge | bac: what's the failing test? | 09:43 |
| bac | thanks | 09:44 |
| bac | henninge: i'll paste it in a sec | 09:44 |
| bac | henninge: when using factory.makePOTemplate it looks like sourcepackagename is non-optional, right? | 09:44 |
| bac | if so that is, uh, dumb | 09:46 |
| bac | henninge: http://pastebin.ubuntu.com/507871/ | 09:48 |
| bac | henninge: the failure is at line 116 | 09:49 |
| bac | and it is barfing on this part of the page template: | 09:49 |
| bac | <a tal:attributes="href context/menu:navigation/templates/url"> | 09:50 |
| bac | full list of templates</a>. | 09:50 |
| bac | templates do not exist | 09:50 |
| bac | henninge: so i think i have not done something that is required in the setup | 09:50 |
| bac | henninge: you still around? | 09:58 |
| henninge | bac: sorry, got called awy. back now | 09:59 |
| bac | np | 09:59 |
| bac | henninge: you can see the failure in devel with | 09:59 |
| bac | bin/test -vvm lp.translations -t TestRobotsDistroSeries | 09:59 |
| bac | if you download that test | 10:00 |
| henninge | the setup looks ok | 10:01 |
| === maxb_ is now known as maxb | ||
| henninge | bac: I wonder if the distribution needs official_rosetta set | 10:06 |
| bac | hmm | 10:06 |
| henninge | but I am still preparing to run the test | 10:06 |
| henninge | bac: can you try ubuntu (but a new distroseries)? | 10:06 |
| bac | henninge: in that test (look at set_usage in the base class) i do set the translation_usage property, which sets official_rosetta | 10:08 |
| henninge | oh, that's what the test is about ... ;-) | 10:10 |
| * henninge is catching on | 10:10 | |
| henninge | bac: which branch is this on? | 10:13 |
| * henninge downloaded the wrong branch | 10:14 | |
| bac | henninge: lp:~bac/launchpad/bug-652280-pg-trans | 10:15 |
| bac | give it a second as i'm pushing | 10:15 |
| bac | and done | 10:18 |
| henninge | branching | 10:19 |
| henninge | bac: ok, I see the failure but am not any wiser yet. | 10:33 |
| bac | henninge: i see the menu item is enabled_with_permission(launchpad.Edit) | 10:34 |
| henninge | bac: ah yes, that is true ;) | 10:34 |
| bac | henninge: the template does not check permissions. but if that is the case i'm not sure how this works at all | 10:35 |
| henninge | bac: can you point me to the template, please | 10:35 |
| bac | distroseries-translations.pt (i think) | 10:35 |
| henninge | sounds sensible | 10:37 |
| henninge | bac: hm, I can see ubuntu/maverick/+templates, even when logged out. | 10:41 |
| bac | yeah, i just saw that | 10:41 |
| henninge | bac: btw, you can do without the test_suite at the end if you turn your "Base" into a "Mixin" | 10:50 |
| bac | henninge: you mean make it not be a TestCase? | 10:51 |
| henninge | yes and make the actual testcases inherit from (TestCase, Mixin) | 10:51 |
| bac | i'm +0 on that. maybe -0 | 10:53 |
| henninge | ;-) | 11:00 |
| henninge | Why is that? | 11:00 |
| henninge | bac: the test has no problems rendering +templates | 11:05 |
| bac | henninge: i don't think actually rendering +templates is an issue, it is using the menu to simply construct the URL | 11:06 |
| * bac baffled | 11:06 | |
| henninge | bac: removing the permissions check from the menu item does not help either. | 11:07 |
| henninge | bac: so clearly it is accessing the wrong menu. | 11:08 |
| bac | henninge: and removing the section from the page template results in another KeyError for language_packs...similar construct | 11:08 |
| henninge | bac: do you know how menu lookup works? | 11:13 |
| henninge | I mean, how "context/menu:..." works? | 11:14 |
| bac | yeah, i think so | 11:15 |
| bac | henninge: it is found through an adaption of the 'usedfor' interface on the given facet | 11:16 |
| bac | henninge: was that sufficiently hand-wavy? | 11:17 |
| henninge | bac: yes, thank you ;) | 11:18 |
| henninge | It just explained why I don't see a connection made in zcml | 11:18 |
| bac | henninge: odd. i did just put a breakpoint in the 'templates' menu item...and it was not hit | 11:18 |
| henninge | bac: makes sense. if the tempalte does not see it it will not call it. | 11:19 |
| bac | duh | 11:20 |
| henninge | bac: I'd ust like to get a "repr" of the menu that is actually used in the template | 11:20 |
| henninge | s/ust/just/ | 11:20 |
| henninge | bac: TranslationUnavailable: Translations for this release series are not available yet. | 11:23 |
| * henninge greps for TranslationUnavailable | 11:23 | |
| bac | huh? | 11:24 |
| bac | where'd that come from? | 11:24 |
| henninge | bac: I commented out both problem sites in the template. | 11:25 |
| henninge | lib/canonical/launchpad/webapp/error.py:class TranslationUnavailableView(SystemErrorView): | 11:25 |
| bac | henninge: so do you see something i'm missing in my setup that would cause translations to not be available? | 11:26 |
| henninge | bac: check_distroseries_translations_viewable | 11:26 |
| henninge | distroseries.hide_all_translations | 11:27 |
| henninge | bac: I'll try setting that to false | 11:27 |
| bac | yes, it is true by default | 11:28 |
| henninge | hmph | 11:29 |
| henninge | bac: no joy :( | 11:29 |
| bac | i made that change to set hide_all_translations=False. now i get a ComponentLookupError on +hierarchy on all tests | 11:34 |
| henninge | bac: oh, really? I just get the old error. But I also reverted all my other changes. | 11:35 |
| henninge | danilos: ! | 11:35 |
| danilos | henninge, hey | 11:36 |
| henninge | danilos: bac created a new distroseseries on a test and tries to render +translations on it but it fails. What could be missing? ;) | 11:38 |
| danilos | henninge, sorry, otp with bigjools | 11:38 |
| henninge | :( | 11:38 |
| henninge | danilos: np | 11:38 |
| bac | henninge: thank you for your help. i must EOD now in bitter defeat. | 12:04 |
| henninge | bac: sorry but tomorrow is s a new day ... ;) | 12:06 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | StevenK: There's no branch for you on launchpad/+activereviews, so is your queue position stale data? | 12:56 |
| * allenap assumes it's stale. | 13:07 | |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: bryceh || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| === Ursinha-afk is now known as Ursinha | ||
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| gmb | allenap: Can I get a review for https://code.edge.launchpad.net/~gmb/launchpad/launchpadformify-subscribe-view-bug-651240/+merge/37850 please? | 14:43 |
| allenap | gmb: Certainly :) | 14:43 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: gmb || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| gmb | Ta.] | 14:45 |
| adeuring | allenap: the changes in your branch look good. But what is the reason for the changes? Do you assume that the SQL query for the old method would be too slow? | 14:46 |
| allenap | adeuring: It's easier to select the targets elsewhere, and it's makes the resulting code closer to the existing structural subs code, so it's easier to reuse. | 14:48 |
| allenap | adeuring: My follow-on branch is now simpler to implement. | 14:48 |
| adeuring | allenap: ok. But now you may get a subscription more than once, if somebody is subscribed to two or more bug targets having bug tasks for a given bug | 14:49 |
| allenap | adeuring: That turns out to be a problem. So far :) | 14:50 |
| allenap | adeuring: That turns out to *not* be a problem. So far :) | 14:50 |
| adeuring | allenap: interesting. But anyway, r=me; the possible duplicate results can be treated later | 14:52 |
| bigjools | wgrant: moving here.... | 14:52 |
| bigjools | wgrant: test_ftparchive.py doesn't seem to test anything? | 14:53 |
| allenap | adeuring: Thanks :) | 14:55 |
| wgrant | bigjools: It tests that a disabled arch doesn't make it into the a-f config. It's from before I had the other tests -- it's redundant now, but more direct. I'm not sure if I should remove it or not. | 14:58 |
| bigjools | wgrant: I can't see any new assertions in it, just setup | 14:59 |
| wgrant | bigjools: The assertion is a-f config file comparison. | 14:59 |
| bigjools | wgrant: I can't see any comparisons | 15:00 |
| wgrant | It's probably a bad place to put it, but the rest of the tests are so black-box that it seems bad to rely on solely on them. | 15:00 |
| bigjools | lines 81-89 in the diff | 15:00 |
| bigjools | sets up the disabled das and re-generates config | 15:01 |
| wgrant | bigjools: I'm adding a new disabled distroarchseries. The point of the test is to show that it doesn't show up in the a-f config file. So the a-f config comparison does not change from the original test. | 15:01 |
| bigjools | wgrant: that's not a good test, since it works with or without that setup | 15:02 |
| wgrant | Right, exactly. | 15:02 |
| wgrant | But I don't see how else to do it. | 15:02 |
| wgrant | Except for the even blacker-box approach of the other test. | 15:02 |
| bigjools | add a direct assertion that it's not in the config | 15:03 |
| wgrant | Eh. I suppose. | 15:04 |
| bigjools | well, that's a bit shit too | 15:04 |
| wgrant | It is. | 15:04 |
| wgrant | This part of the publisher is, sadly, shit. | 15:04 |
| wgrant | And the tests are worse. | 15:05 |
| bigjools | so, we need a test that it does appear when enabled, and not there when disabled | 15:05 |
| wgrant | Ideally. I could just drop this test, since it's covered by the other one now. | 15:05 |
| bigjools | true | 15:05 |
| bigjools | they are better tests | 15:06 |
| bigjools | since you have negative and positive | 15:06 |
| wgrant | They are. But it feels a little awkward to be testing this behaviour only at that very high level. | 15:06 |
| wgrant | But I guess the code leaves us little choice. | 15:06 |
| bigjools | my only comment about them is that they do a lot of asserting and if one fails you won't see the results of the others | 15:07 |
| bigjools | so if you can think of a way to assert all at once that would be better | 15:07 |
| wgrant | That would certainly be a good thing. But I don't know of any easy way. | 15:07 |
| wgrant | So, I'll drop the test_ftparchive test. | 15:08 |
| bigjools | one way is to make two lists and compare them | 15:08 |
| wgrant | I would love to improve the test, but we sort of need this code soon, and I need to leave for uni in a little over 6 hours. So I might not. | 15:10 |
| bigjools | that's ok | 15:10 |
| bigjools | ditch the a-f test then | 15:11 |
| bigjools | and I'll land the branch | 15:11 |
| bigjools | set the MP to "needs review" though :) | 15:11 |
| wgrant | Pushing... | 15:12 |
| bigjools | stab stab stab dogfood | 15:13 |
| wgrant | It's still going? | 15:13 |
| bigjools | yes. perhaps it needs more coal. | 15:13 |
| wgrant | It seriously cannot take that long to dump 100000 rows from a view. | 15:14 |
| wgrant | Or are there other dirty pockets? | 15:14 |
| bigjools | well, it's chugging on lucid. Perhaps I should restart with -s maverick | 15:15 |
| bigjools | :/ | 15:15 |
| wgrant | That may be a reasonable idea. | 15:15 |
| bigjools | le sigh | 15:15 |
| wgrant | bigjools: Diff updated. I am happy enough with the branch now. | 15:16 |
| bigjools | ok | 15:17 |
| bigjools | wgrant: I'll test and land, thanks for writing it | 15:17 |
| wgrant | Thanks. | 15:18 |
| wgrant | A little unfortunate that I couldn't avoid that wonderful [7:] | 15:18 |
| bigjools | shrug :/ | 15:19 |
| wgrant | But I tried to block it in a few other places, and they wouldn't work, so... | 15:19 |
| bigjools | so it was quicker this time | 15:21 |
| bigjools | the i386 Packages didn't get re-written though :/ | 15:22 |
| bigjools | I'll try again, the ctrl-c may have done nasty stuff | 15:22 |
| wgrant | Ugh. | 15:23 |
| wgrant | There'll be no dirty pockets. | 15:23 |
| wgrant | You'll need to flip one of the pubs back to Pending. | 15:23 |
| bigjools | yes | 15:23 |
| bigjools | or upload a new package, which is a better test | 15:23 |
| wgrant | But that'll take another 10 hours. | 15:23 |
| wgrant | Anyway, I should sleep. | 15:23 |
| bigjools | nope | 15:23 |
| wgrant | Good luck with that... | 15:23 |
| bigjools | 5 mins | 15:24 |
| bigjools | thanks wgrant | 15:24 |
| wgrant | Thanks for fixing my rather spectacular fuckup. | 15:24 |
| bigjools | it's not quite done yet, we need to send grovelling emails to PPA owners | 15:24 |
| bigjools | but enough of making you feel bad for one day :) | 15:24 |
| bigjools | g'night | 15:25 |
| wgrant | Most of them are probably already ancient, unnoticed history. | 15:25 |
| wgrant | But we need to check that out, yeah. | 15:25 |
| wgrant | Night. | 15:25 |
| gmb | allenap: How's that review looking? It contains a fix for some of the current buildbot breakage so the sooner it's RTL the better. | 15:30 |
| gmb | (I'll split that bit out if necessary, though). | 15:30 |
| allenap | gmb: I'm about half way through it, but it's going quickly. I'll put the pedal to the floor though :) | 15:31 |
| gmb | allenap: Ta. | 15:31 |
| sinzui | gmb, was the test wrong? is the subscription to mozilla-firefox (Ubuntu)? | 15:42 |
| gmb | sinzui: Well, I'm just looking now, and I'm getting the opposite results locally. It should be to Mozilla Firefox, not mozilla-firefox (Ubuntu) because the Mozila Firefox task is the primary task for the bug and we subscribe to the bug not the task. | 15:43 |
| sinzui | ah | 15:43 |
| gmb | sinzui: So, the test passes as it is in stable on my local machine. Change it to fix the BB failure and it fails locally. | 15:43 |
| gmb | I think my brain may melt out through my ears shortly. | 15:45 |
| allenap | gmb: Sorry, I fscked up your review by missing the prerequisite branch. Approved though. | 15:52 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| gmb | allenap: Thanks. | 15:52 |
| gmb | allenap: So _subscribers_for_current_user is used quite a bit in the subsequent branch, so I'm happier to leave that as-is. | 15:53 |
| gmb | allenap: And _isSubscriptionRequest() is vestigial and no longer in use. | 15:54 |
| allenap | gmb: Yeah, those comments of mine need a lot of salt today. | 15:54 |
| gmb | (Which I just noticed) | 15:54 |
| gmb | allenap: Fair enough. I'll remove one and leave the other, then. | 15:54 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: - || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| noodles775 | Hi allenap, do you have time for https://code.edge.launchpad.net/~michael.nelson/launchpad/656166-expose-dsd-diffs/+merge/37858 | 16:02 |
| allenap | noodles775: otp, but sure. | 16:02 |
| noodles775 | Thanks. | 16:03 |
| * noodles775 fixes conflicts | 16:05 | |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: noodles775 || queue: [allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === deryck is now known as deryck[lunch] | ||
| gmb | jml: I have a fix for the xx-person-subscriptions failure if you'd like to cast a weather eye over it; https://code.edge.launchpad.net/~gmb/launchpad/stable-unfark/+merge/37860 | 16:14 |
| jml | gmb: thanks. looking now. | 16:16 |
| noodles775 | allenap: ok, the MP is now updated and without conflicts. | 16:21 |
| allenap | noodles775: Cool. | 16:22 |
| === matsubara is now known as matsubara-lunch | ||
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: - || queue: [allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === deryck[lunch] is now known as deryck | ||
| === salgado is now known as salgado-lunch | ||
| === allenap 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 | ||
| === matsubara-lunch is now known as matsubara | ||
| === salgado-lunch is now known as salgado | ||
| === gary_poster_ is now known as gary_poster | ||
| === benji is now known as benji-lunch | ||
| sinzui | EdwinGrubbs, ping | 19:20 |
| sinzui | EdwinGrubbs, do you have time to review | 19:22 |
| sinzui | https://code.launchpad.net/~sinzui/launchpad/mlist-sync-0/+merge/37885 that fixes an oops? | 19:22 |
| EdwinGrubbs | sinzui: sure | 19:23 |
| === salgado is now known as salgado-afk | ||
| EdwinGrubbs | sinzui: r=me | 19:50 |
| sinzui | thanks EdwinGrubbs | 19:51 |
| === benji-lunch is now known as benji | ||
| === EdwinGrubbs is now known as Edwin-lucnh | ||
| === Edwin-lucnh is now known as Edwin-lunch | ||
| === ajmitch_ is now known as ajmitch | ||
| === matsubara is now known as matsubara-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!