[03:17] <sinzui> Edwin: I approved your branch
[03:44] <EdwinGrubbs> benji: r=me
[03:44] <EdwinGrubbs> sinzui: thanks
[05:14] <EdwinGrubbs> StevenK: are you a reviewer?
[05:17] <StevenK> EdwinGrubbs: I am, but my mentor is on holidays
[05:17]  * mwhudson can mentor something if it's easy
[05:18] <mwhudson> (not because i'm lazy but because i need to disappear fairly soon)
[05:19] <EdwinGrubbs> StevenK: well, I want you to look at the branch since it is a soyuz testfix for db-devel.
[05:20] <StevenK> EdwinGrubbs: Sure, point me to the MP?
[05:24] <EdwinGrubbs> StevenK: here it is: https://code.edge.launchpad.net/~edwin-grubbs/launchpad/broken_test_do_not_copy_disabled_dases/+merge/37815
[05:24] <StevenK> Oh, that's my fault :-/
[05:25] <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:29] <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:30] <EdwinGrubbs> StevenK: you can just fix it.
[05:32] <StevenK> EdwinGrubbs: Okay, doing so, thanks.
[06:53] <wgrant> StevenK: Quick Soyuz one, if you're interested: https://code.edge.launchpad.net/~wgrant/launchpad/bug-655648-a-f-maverick/+merge/37820
[06:55] <StevenK> wgrant: Added my code* to it
[06:57] <wgrant> StevenK: Thanks.
[07:47] <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:59]  * stub has a look
[08:06] <stub> noodles775: Might we need to join or lookup rows using base_version ? I'm wondering if it needs an index or not.
[08:10] <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:21] <stub> noodles775: Sounds like we don't need it.
[08:23] <stub> noodles775: r=stub patch-2208-24-0.sql
[08:23] <noodles775> Great, thanks stub!
[09:28] <bac> hi henninge
[09:35] <henninge> Hi bac!
[09:36]  * henninge looks at activereviews
[09:36] <bac> hi henninge, could you have another look at my MP?  sinzui did the UI mentor
[09:37] <bac> henninge: i'm also having some trouble with a translation test and need some domain-specific geniusity
[09:40] <henninge> oh, is there a planned downtime for code hosting atm?
[09:41] <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:43] <henninge> bac: r=me
[09:43] <henninge> bac: what's the failing test?
[09:44] <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:46] <bac> if so that is, uh, dumb
[09:48] <bac> henninge: http://pastebin.ubuntu.com/507871/
[09:49] <bac> henninge: the failure is at line 116
[09:49] <bac> and it is barfing on this part of the page template:
[09:50] <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:58] <bac> henninge: you still around?
[09:59] <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
[10:00] <bac> if you download that test
[10:01] <henninge> the setup looks ok
[10:06] <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:08] <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:10] <henninge> oh, that's what the test is about ... ;-)
[10:10]  * henninge is catching on
[10:13] <henninge> bac: which branch is this on?
[10:14]  * henninge downloaded the wrong branch
[10:15] <bac> henninge: lp:~bac/launchpad/bug-652280-pg-trans
[10:15] <bac> give it a second as i'm pushing
[10:18] <bac> and done
[10:19] <henninge> branching
[10:33] <henninge> bac: ok, I see the failure but am not any wiser yet.
[10:34] <bac> henninge: i see the menu item is enabled_with_permission(launchpad.Edit)
[10:34] <henninge> bac: ah yes, that is true ;)
[10:35] <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:37] <henninge> sounds sensible
[10:41] <henninge> bac: hm, I can see ubuntu/maverick/+templates, even when logged out.
[10:41] <bac> yeah, i just saw that
[10:50] <henninge> bac: btw, you can do without the test_suite at the end if you turn your "Base" into a "Mixin"
[10:51] <bac> henninge: you mean make it not be a TestCase?
[10:51] <henninge> yes and make the actual testcases inherit from (TestCase, Mixin)
[10:53] <bac> i'm +0 on that.  maybe -0
[11:00] <henninge> ;-)
[11:00] <henninge> Why is that?
[11:05] <henninge> bac: the test has no problems rendering +templates
[11:06] <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:07] <henninge> bac: removing the permissions check from the menu item does not help either.
[11:08] <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:13] <henninge> bac: do you know how menu lookup works?
[11:14] <henninge> I mean, how "context/menu:..." works?
[11:15] <bac> yeah, i think so
[11:16] <bac> henninge: it is found through an adaption of the 'usedfor' interface on the given facet
[11:17] <bac> henninge: was that sufficiently hand-wavy?
[11:18] <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:19] <henninge> bac: makes sense. if the tempalte does not see it it will not call it.
[11:20] <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:23] <henninge> bac: TranslationUnavailable: Translations for this release series are not available yet.
[11:23]  * henninge greps for TranslationUnavailable
[11:24] <bac> huh?
[11:24] <bac> where'd that come from?
[11:25] <henninge> bac: I commented out both problem sites in the template.
[11:25] <henninge> lib/canonical/launchpad/webapp/error.py:class TranslationUnavailableView(SystemErrorView):
[11:26] <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:27] <henninge> distroseries.hide_all_translations
[11:27] <henninge> bac: I'll try setting that to false
[11:28] <bac> yes, it is true by default
[11:29] <henninge> hmph
[11:29] <henninge> bac: no joy :(
[11:34] <bac> i made that change to set hide_all_translations=False.  now i get a ComponentLookupError on +hierarchy on all tests
[11:35] <henninge> bac: oh, really? I just get the old error. But I also reverted all my other changes.
[11:35] <henninge> danilos: !
[11:36] <danilos> henninge, hey
[11:38] <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
[12:04] <bac> henninge: thank you for your help.  i must EOD now in bitter defeat.
[12:06] <henninge> bac: sorry but tomorrow is s a new day ... ;)
[12:56] <allenap> StevenK: There's no branch for you on launchpad/+activereviews, so is your queue position stale data?
[13:07]  * allenap assumes it's stale.
[14:43] <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:45] <gmb> Ta.]
[14:46] <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:48] <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:49] <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:50] <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:52] <adeuring> allenap: interesting. But anyway, r=me; the possible duplicate results can be treated later
[14:52] <bigjools> wgrant: moving here....
[14:53] <bigjools> wgrant: test_ftparchive.py doesn't seem to test anything?
[14:55] <allenap> adeuring: Thanks :)
[14:58] <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:59] <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.
[15:00] <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:01] <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:02] <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:03] <bigjools> add a direct assertion that it's not in the config
[15:04] <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:05] <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:06] <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:07] <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:08] <wgrant> So, I'll drop the test_ftparchive test.
[15:08] <bigjools> one way is to make two lists and compare them
[15:10] <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:11] <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:12] <wgrant> Pushing...
[15:13] <bigjools> stab stab stab dogfood
[15:13] <wgrant> It's still going?
[15:13] <bigjools> yes.  perhaps it needs more coal.
[15:14] <wgrant> It seriously cannot take that long to dump 100000 rows from a view.
[15:14] <wgrant> Or are there other dirty pockets?
[15:15] <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:16] <wgrant> bigjools: Diff updated. I am happy enough with the branch now.
[15:17] <bigjools> ok
[15:17] <bigjools> wgrant: I'll test and land, thanks for writing it
[15:18] <wgrant> Thanks.
[15:18] <wgrant> A little unfortunate that I couldn't avoid that wonderful [7:]
[15:19] <bigjools> shrug :/
[15:19] <wgrant> But I tried to block it in a few other places, and they wouldn't work, so...
[15:21] <bigjools> so it was quicker this time
[15:22] <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:23] <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:24] <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:25] <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:30] <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:31] <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:42] <sinzui> gmb, was the test wrong? is the subscription to mozilla-firefox (Ubuntu)?
[15:43] <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:45] <gmb> I think my brain may melt out through my ears shortly.
[15:52] <allenap> gmb: Sorry, I fscked up your review by missing the prerequisite branch. Approved though.
[15:52] <gmb> allenap: Thanks.
[15:53] <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:54] <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.
[16:02] <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:03] <noodles775> Thanks.
[16:05]  * noodles775 fixes conflicts
[16:14] <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:16] <jml> gmb: thanks. looking now.
[16:21] <noodles775> allenap: ok, the MP is now updated and without conflicts.
[16:22] <allenap> noodles775: Cool.
[19:20] <sinzui> EdwinGrubbs, ping
[19:22] <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:23] <EdwinGrubbs> sinzui: sure
[19:50] <EdwinGrubbs> sinzui: r=me
[19:51] <sinzui> thanks EdwinGrubbs