[00:25] <EdwinGrubbs> sinzui: did you already test your query's performance on staging?
[00:25] <sinzui> No. I have not
[00:26] <sinzui> I am happy to test rather than land this and fail it in QA
[00:26]  * sinzui has already failed the first effort
[00:26] <sinzui> EdwinGrubbs: I am trying to force the data to look like the old test. I have had no success.
[00:27] <EdwinGrubbs> sinzui: according to launchpad_dev, DistroSeriesPackageCache.name does not have an index. It would be good to test the query with and without the index on staging.
[00:27] <sinzui> oh wow
[00:27] <sinzui> We search on that. I guess we only search on the fti field
[00:27] <sinzui> okay. I will test before considering a landing
[00:32] <lfaraone> EdwinGrubbs: pushed up rev 10009 on lp:~lfaraone/launchpad/bugfix494055, it should fix your concerns.
[00:50] <EdwinGrubbs> lfaraone: looking at it now
[01:49]  * thumper looks for a reviewer
[01:49] <thumper> EdwinGrubbs: still on?
[01:50] <EdwinGrubbs> thumper: well, if it's really small I can take it.
[01:50] <thumper> EdwinGrubbs: heh, it isn't but it is pretty cool
[01:50] <thumper> EdwinGrubbs: feel free to pass for now
[01:51] <EdwinGrubbs> thumper: sorry, my day really ended a while ago.
[01:52] <sinzui> thumper: I can do in a few minutes
[01:54] <thumper> sinzui: awesome - https://code.edge.launchpad.net/~thumper/launchpad/revisions-in-conversation/+merge/15910
[01:54] <thumper> sinzui: the diff should be arriving shortly
[01:55] <sinzui> fab
[01:55] <thumper> sinzui: it is there now
[01:57] <thumper> sinzui: I'm just popping out to collect the girls from school
[01:57] <thumper> sinzui: back shortly
[02:00] <EdwinGrubbs> lfaraone: your test fix looks good. I just am experiencing a weird error in the test data. I will land your branch tomorrow.
[02:10] <lfaraone> EdwinGrubbs: cool, thanks. 
[02:11] <thumper> sinzui: back
[02:39] <thumper> sinzui: are you checking it out?
[02:39] <thumper> sinzui: I have to head off in about 5-10 minutes
[02:40] <thumper> sinzui: so any questions you have, ask now
[02:40] <sinzui> thumper: I am reading it, but my son insists on a new diaper
[02:40] <thumper> heh
[07:31] <adiroiban> abentley: hi. is there anything more I need to do for having this branch landed: https://code.edge.launchpad.net/~adiroiban/launchpad/bug-431249/+merge/15767
[09:26] <danilos> adiroiban, btw, I've sent you another review for that permissions branch
[09:26] <danilos> adiroiban, I can also help you land bug-431249 if you want me to :)
[09:27] <adiroiban> danilos: thanks . I'll look at it
[09:27] <adiroiban> feel free to land that branch :)
[09:27] <danilos> adiroiban, there's still some work to be done on it, but I am around for any questions (it's probably going to be faster if you ask me here :)
[09:28] <adiroiban> danilos: ok
[09:40] <danilos> adiroiban, ok, I am landing your branch as well
[09:40] <adiroiban> danilos: thanks. still reading the review :)
[09:41] <danilos> adiroiban, heh, right :)
[10:12] <adiroiban> danilos: how is the inheritance working for classes from security.py ? I think's I'm missing a python mechanism
[10:13] <adiroiban> checkAuthenticated return True 
[10:13] <adiroiban> and if it returns None, it will call super ?
[10:14] <danilos> adiroiban, well, it's working just like everywhere else... it inherits the methods code, allows overriding, but if you call super-class methods and pass in "self", you are calling super-class methods on current object, which must be compatible then
[10:15] <danilos> adiroiban, you have to call super-class checkAuthenticated manually
[10:15] <adiroiban> ok
[10:15] <adiroiban> I don't understand why you need to use inheritance
[10:16] <adiroiban> since you can call that method anyways
[10:16] <danilos> adiroiban, well, you don't, it's just how most other classes in there do it
[10:16] <danilos> adiroiban, you can use AuthorizationBase as the base class everywhere, and just construct whatever other classes you need
[10:17] <adiroiban> I'm looking at class AdminDistributionTranslations(OnlyRosettaExpertsAndAdmins, EditDistributionByDistroOwnersOrAdmins):
[10:17] <danilos> adiroiban, I'd say that's a much cleaner approach where you pass in only the relevant information to the auth class, but the current approach seems a de-facto standard
[10:18] <adiroiban> and I wonder why it is not AdminDistributionTranslations(AuthorizationBase):
[10:19] <danilos> adiroiban, it's just a pattern which works the same if you are not checking self.obj (i.e. context) in any of the permissions you are deriving from; it keeps the code shorter
[10:19] <adiroiban> ok
[10:21] <danilos> adiroiban, it's just a trick which involves knowledge of python OO model :)
[10:37] <adeuring> noodles775: fancy a small ui/code review: https://code.edge.launchpad.net/~adeuring/launchpad/bug-172507/+merge/15925 ? (the real diff is two added/changed lines, the remaining 1000 lines are just an added SVG file ;)
[10:38] <noodles775> adeuring: Sure! I'll do the ui review, but you'll need to grab someone else for the code (beuno's policy about not doing both code/ui for the one review)
[10:38] <adeuring> noodles775: yeah... but please have a look at the "code" first ;)
[10:39] <noodles775> ah, ok, it's like that then :-)
[10:39] <noodles775> No worries.
[10:40] <noodles775> I see, literally two lines! It's mine I tell you!
[10:52] <bigjools> al-maisan or noodles775, do you want to review my CP candidate
[10:52] <al-maisan> bigjools: I can do it if noodles775 is busy
[10:53] <al-maisan> bigjools: will do.
[10:53] <noodles775> ta, I'll do adeuring's branch.
[10:53] <bigjools> thanks guys
[10:54] <bigjools> al-maisan: ah crap
[10:54] <al-maisan> ?
[10:54] <bigjools> I need to push another revision, I left a stale method, ignore the "assertNoBuildQueue" test
[10:55] <al-maisan> bigjools: no problem -- just let me know when it's done.
[10:55] <bigjools> diff will regenerate itself
[11:01] <bigjools> al-maisan: https://code.edge.launchpad.net/~julian-edwards/launchpad/die-buildqueue-die--bug-492632/+merge/15927
[11:02] <al-maisan> bigjools: thanks!
[11:03] <bigjools> grrrr the person chooser on the MPs gives you no chance to review what you clicked on
[11:04] <bigjools> BjornT_: can you review the above MP for r-c please
[11:13] <al-maisan> bigjools: r=me
[11:15] <bigjools> ta
[11:29] <noodles775> Sorry adeuring - I should have just typed it here... I think you forgot to update the icon in the comment itself? (see the screenshot linked from my comment on the MP)
[11:30] <adeuring> noodles775: right, just gor remonder from deryck :) I'm adding that now
[11:30] <noodles775> Great.
[11:31] <danilos> al-maisan, another branch in the queue :) noodles775, or you if you fancy a short OOPS fix :)
[11:31] <danilos> https://code.edge.launchpad.net/~danilo/launchpad/bug-480652/+merge/15931
[11:32] <noodles775> danilos: yeah, I'll take it (I think al-maisan is on lunch)
[11:32] <danilos> noodles775, cool, thanks
[11:38] <adiroiban> danilos: how should we display an inactive-template, what css atributes should be changed ? 
[11:38] <noodles775> beuno: have you got time to take a peek at a screenshot? https://bugs.edge.launchpad.net/soyuz/+bug/211008/comments/1
[11:39] <mup> Bug #211008: Add visual indicator for obsolete (superseded) packages in PPA <oem-services> <ppa> <ui> <Soyuz:In Progress by michael.nelson> <https://launchpad.net/bugs/211008>
[11:39] <danilos> adiroiban, I am not exactly sure, perhaps we can ask noodles775 for an idea? :)
[11:40] <noodles775> is there a bug? or on which page?
[11:40] <adiroiban> https://translations.launchpad.dev/ubuntu/hoary/+templates
[11:40] <danilos> noodles775, we've got a big list of templates (in a table) where some of them are inactive (we are showing them for admins only); what'd be your suggestion to do that on eg. https://translations.launchpad.dev/ubuntu/karmic/+templates page
[11:40] <danilos> s/karmic/hoary/ of course, thanks adiroiban :)
[11:41] <bigjools> noodles775: should it go in the +packages page instead?
[11:41] <danilos> noodles775, also https://translations.edge.launchpad.net/openobject-addons/trunk/+templates
[11:41] <noodles775> bigjools: I was going to put it on both.
[11:41] <noodles775> What do you think?
[11:41] <bigjools> ah fair enough
[11:41] <adiroiban> danilos: maybe I can leave the css for the cleanout task
[11:42] <bigjools> yes that sounds good - I'd be tempted to write the version that supersedes it as well
[11:42] <noodles775> aha
[11:42] <adiroiban> cleanup
[11:43] <danilos> adiroiban, at least 'color: grey' should do for now, or a light background
[11:44] <adiroiban> color: grey ... I think that any other background will just hilight the row 
[11:44] <noodles775> adiroiban: How can I set one of the templates on that page to inactive? there's no option... or is it the 'Accept translations' admin option, or something in your branch?
[11:44] <adiroiban> noodles775: the first one disabled-template is not active
[11:44] <adiroiban> yes
[11:45] <adiroiban> is "Accept translations" 
[11:45] <adiroiban> but right now there is no style or class for it
[11:45] <noodles775> adiroiban: Right, but is the behavior affected at all in the UI (ie. I can still edit/upload/download etc. it seems?)
[11:45] <adiroiban> yes
[11:46] <adiroiban> the idea is to give a hint about the state of a template
[11:46] <adiroiban> just a small one
[11:46] <adiroiban> only admins will see those templates that are not active
[11:46] <noodles775> ok, and those templates are only visible to admins.
[11:47] <adiroiban> yep
[11:47] <noodles775> Yep, I personally wouldn't go for color: gray - as that indicates to me that it's disabled (ie. I can't interact with it), but everything in the ui tells me I still can. Hrm.
[11:48] <danilos> noodles775, you can interact with it only as an admin... perhaps we can use an icon or something, but I'd rather just use a CSS style
[11:49] <danilos> noodles775, at the moment, set of users who deal with these (i.e. who get to see disabled templates) is very small (LP translations team + Ubuntu Translations coordination team, total ~10 people)
[11:49] <noodles775> danilos: yeah, so the person who sees that it is grayed out can interact with it. I was wondering whether it might be simpler to say Template name: disabled-template (inactive)
[11:50] <danilos> noodles775, that would be harder to notice in a big list IMHO; how about a different TR background?
[11:50] <danilos> noodles775, not that that wouldn't work, I just want to make it more visible
[11:50] <danilos> well, I want adiroiban to make it more visible :))
[11:50] <noodles775> danilos: yeah, in a big list something like that would be good.
[11:50] <noodles775> So
[11:51] <adiroiban> :D
[11:51] <noodles775> if the background color of the row is shaded slightly and, so that people don't think, 'huh, why is it shaded', we indicate the inactive state in the name as above?
[11:51] <danilos> noodles775, +1
[11:51] <danilos> adiroiban, how do you like that? :)
[11:52] <adiroiban> it's ok
[11:52] <adiroiban> i'll do it :)
[11:52] <danilos> adiroiban, if you have any better/different suggestions, feel free to tell them :)
[11:52] <noodles775> adiroiban: feel free to do something that you like more...
[11:52] <danilos> noodles775, adiroiban: cool, thanks!
[11:52] <noodles775> +`
[11:52] <noodles775> +1
[11:53] <adiroiban> uh... color ?
[11:53] <adiroiban> colorcode
[11:54] <adiroiban> #DCDCDC
[11:54]  * noodles775 looks in style-3.0.css
[11:55] <noodles775> #747474 is what's being used for a disabled tab, but it might be too dark. See what you think.
[11:56] <adiroiban> nope. to dark
[11:56] <adiroiban> that was my starting point 
[11:59] <adiroiban> let me try some colors and show you some screenshots
[12:01] <danilos> adiroiban, don't worry about it too much, if it works for you, it'll work for the remaining 9 people who are going to see it :)
[12:01] <adiroiban> ok
[12:06] <adiroiban> danilos: should I rename the other xx-rosetta-pofile stories to xx-pofile?
[12:07] <danilos> adiroiban, sure
[12:08] <danilos> adiroiban, again, some of it may break tests (i.e. some tests are loaded through python files), so you don't have to, but if you want to fight more problems... :)
[12:09] <adiroiban> danilos: or they can create name conflicts?
[12:09] <danilos> adiroiban, yeah, that too
[12:09] <adiroiban> with stories from other products?
[12:10] <danilos> adiroiban, so, if you want to get your branch in, I wouldn't worry about fixing all the worlds problems in it :)
[12:10] <adiroiban> :)
[12:31] <beuno> noodles775, still need that peak?
[12:32] <noodles775> beuno: yeah, if you wouldn't mind, then I can finish it and put an MP together :-) Thanks!
[12:36] <beuno> noodles775, where does "newer version" link?
[12:37] <noodles775> beuno: https://launchpad.dev/ubuntu/warty/+source/iceweasel/1.1
[12:38] <beuno> noodles775, tha tonly thing that worries me a bit here
[12:38] <beuno> is that if a PPA is there for say, backports, you'll have that on every single row
[12:38] <BjornT_> bigjools: is there any test for code that calls BuildQueue.destroySelf()?
[12:39] <noodles775> beuno: Erm, I don't think so, one tick...
[12:39] <bigjools> BjornT_: I dunno without looking
[12:40] <danilos> noodles775, I am out for lunch: my branch should be pretty simple, but if you've got any questions, feel free to tackle somebody else's branch so we can deal with mine when I am back :) thanks
[12:40] <noodles775> beuno: actually, yes, I think you're right. As long as it's in one of the distributions archives.
[12:40] <bigjools> BjornT_: the best we have is lib/lp/soyuz/doc/buildd-slavescanner.txt
[12:41] <noodles775> danilos: yes, it looks straight forward, sorry, I've just had other things coming up.
[12:41] <danilos> noodles775, don't worry, it's not a huge rush, when you get to it, great :)
[12:42] <beuno> noodles775, in light of that, what do you think about making it less prominent?
[12:43] <noodles775> beuno: sure (I just checked, anything published in primary, partner or debug is included).
[12:44] <beuno> noodles775, it seems more informational than a warning in this case
[12:44] <BjornT_> bigjools: why does it call destroySelf() in the test itself? what code actually calls destroySelf() in production?
[12:45] <noodles775> beuno: yep, I'll switch the icon, but other than that, I'm not sure how to make it less prominent while still conveying enough info (and a link) to identify what it is?
[12:45] <BjornT_> bigjools: btw, on line 514 you try to call destroySelf(), but the isn't prefixed with >>>
[12:46]  * beuno thinks
[12:46] <bigjools> BjornT_: heh, there's a few without the >>>
[12:46] <bigjools> that's an old old test
[12:47] <bigjools> BjornT_: see the bottom few methods of lib/lp/buildmaster/buildergroup.py
[12:47] <bigjools> that's the code that handles builds returned from the buildds
[12:49] <beuno> noodles775, maybe less verbose?  "(i) _Newer version_ available"
[12:50] <noodles775> beuno: OK - I'd thought it necessary to somehow indicate that it was a newer version in the distroseries itself. But you're right. I'll go with that. Thanks!
[12:51] <BjornT_> bigjools: ok. i assume that you have, or will test this patch on dogfood?
[12:51] <bigjools> BjornT_: I have yes - it's the right thing to do anyway, it was omitted in the recent refactoring
[12:51] <beuno> noodles775, something better may come to mind once the caffeine starts flowing to my brain  :)
[12:51] <salgado> BjornT_, care to review a trivial lazr-js branch for me? https://code.edge.launchpad.net/~salgado/lazr-js/bug-482235/+merge/15940
[12:54] <BjornT_> salgado: sure
[12:54] <salgado> BjornT_, in fact, I'm more interested in feedback about the feature itself, as I'll ask mars for another review later, when he's around
[12:55] <BjornT_> salgado: ok
[13:00] <BjornT_> salgado: can you give an example in LP code, where you would use this? i.e., where we use the Picker, and where you plan to specify this new argument
[13:01] <salgado> BjornT_, the vocabulary picker
[13:01] <salgado> in Launchpad
[13:01] <salgado> I enter a person's name but am not sure whether or not it's correct, so I click 'Choose...' to search for it
[13:02] <salgado> the overlay pops up and I have to type the name again
[13:02] <BjornT_> salgado: can you point me to a file? i want to see how it's connected to the text field
[13:04] <salgado> BjornT_, canonical/widgets/templates/inline-picker.pt
[13:06] <BjornT_> salgado: not vocabulary-picker.js?
[13:09] <salgado> BjornT_, could be; that's not quite clear to me
[13:11] <BjornT_> salgado: yeah. i'll add a comment to the MP. i have some thoughts on the API
[13:13] <bigjools> BjornT_: interesting point about the attributes in my test
[13:13] <bigjools> I'll change it to use another variable "just in case"
[13:13] <bigjools> it'll be the fifth time I've re-written the test :)
[13:14] <BjornT_> bigjools: yeah, it's better to be on the safe side
[13:15] <bigjools> I agree
[13:35] <salgado> BjornT_, did you forget about that comment to the MP?
[13:39] <BjornT_> salgado: no, just got distracted :) it's there now
[13:39] <adiroiban> danilos: the branch with changes to launchpad.TranslationsAdmin privileges is not ready. It looks like I need to also look at the db schema
[13:48] <danilos> adiroiban, why's that?
[13:49] <adiroiban> danilos: http://paste.ubuntu.com/338672/
[13:50] <adiroiban> danilos: I assume that as an UTC i only got an launchpad.TranslationsAdmin "token"
[13:51] <adiroiban> danilos: maybe I'm talking nonsenses
[13:51] <danilos> adiroiban, I am trying to look it up, my connection to canonical servers is slow today, not sure why :(
[13:53] <danilos> adiroiban, that's right, it just means that you missed a spot where you should have changed the permission to TranslationsAdmin instead of Admin, are all your latest changes in a branch I can take a look at?
[13:53] <adiroiban> yes. in the MP one
[13:54] <danilos> adiroiban, cool, let me get it here
[13:55] <adiroiban> danilos: here is a "clean" error http://paste.ubuntu.com/338675/
[13:56] <adiroiban> danilos: you get that when trying to save a template as UTC
[14:01] <danilos> adiroiban, look at translations/configure.zcml, search for POTemplate, and look what launchpad.Admin is required for
[14:01] <danilos> adiroiban, changing that to launchpad.TranslationsAdmin makes the test pass for me
[14:04] <adiroiban> danilos: thanks. I was looking only at translations/browser/configure.zcml
[14:07] <danilos> adiroiban, np
[14:08] <danilos> adiroiban, btw, this change will introduce some more privileges to you guys, you'll get some privileges on language pack pages and such
[14:12]  * danilos switches locations
[14:27] <henninge> hi al-maisan, noodles775:  I added my name to the queue for a branch that started out as a "quick fix" but got a bit longer because of tests. The original reviewer asked if somebody else could finish this.
[14:27] <henninge> noodles775: are you doing UI exclusively?
[14:27] <al-maisan> henninge: where is that branch?
[14:27] <henninge> al-maisan: https://code.edge.launchpad.net/~henninge/launchpad/access-to-translationgroup-table/+merge/15643
[14:27]  * al-maisan looks
[14:28] <noodles775> henninge: if possible, only because I've already done lots of UI reviews this week, so keen to minimise any more time spent reviewing.
[14:28] <al-maisan> henninge: I'll take care of it.
[14:28] <henninge> noodles775: ok, I was just curious, und außerdem ^
[14:28] <henninge> al-maisan: Danke schön!
[14:29] <al-maisan> henninge: Bitte, nichts zu danken :)
[14:40] <allenap> noodles775: Fancy doing a UI review of https://code.edge.launchpad.net/~allenap/launchpad/show-me-too-counts/+merge/15947? I haven't got screen-shots, but can get some if you'd like.
[14:40] <noodles775> allenap: np, I'll want to run it anyway.
[14:41] <allenap> noodles775: Cool, thanks :)
[14:41] <noodles775> allenap: np. I'll take a look in 15m or so.
[14:42] <al-maisan> henninge: quick question: should the new/revised tests in test_translationimportqueue.py fail if the security.cfg changes are not applied to the database?
[14:42] <henninge> al-maisan: yes, some of them.
[14:43] <henninge> three, I think
[14:43] <henninge> one for each of the three tables
[14:43] <allenap> al-maisan: Are you up for a code and/or js review of https://code.edge.launchpad.net/~allenap/launchpad/show-me-too-counts/+merge/15947?
[14:44] <henninge> allenap: do we still split js and code?
[14:45] <allenap> henninge: I don't know... I got a merge proposal yesterday with separate code and js reviews.
[14:45] <allenap> henninge: Most if not all of us are js reviewers, so I guess it makes little sense.
[14:45] <al-maisan> allenap: I could do the code review once I am done with henninge's branch .. how does that sound?
[14:46] <al-maisan> allenap: if needs be I can look at the js piece as well.
[14:46] <allenap> al-maisan: That's great, thanks.
[14:46] <henninge> allenap: I never formally graduated as js reviewer but I think to be remembering that at one point it was said that all reviewers can do js reviews now.
[14:47] <adeuring> noodles775: http://people.canonical.com/~adeuring/patch-icons1.png
[14:47] <noodles775> Great, thanks adeuring !
[14:47] <allenap> henninge: Okay, perhaps something to bring up on the list or at the next reviewer meeting?
[14:48] <henninge> allenap: yup, next reviewers meeting on the 6th.
[14:50] <adeuring> noodles775: just noticed that it is trivial to enable the "patch" checkbox in the "add attachment" form. Let me add that too
[14:50] <noodles775> adeuring: that'd be great!
[14:51] <henninge> allenap: https://dev.launchpad.net/ReviewerMeetingAgenda
[14:51] <allenap> henninge: Fantastic :)
[14:55] <adiroiban> danilos: hi. I pushed the latest change, and after running bin/test -vvc -m translations --layer=PageTestLayer , everhing seems fine.
[15:00] <danilos> adiroiban, cool, sounds good, I'll take a look at it asap
[15:01] <adiroiban> danilos: there's no hurry :)
[15:09] <al-maisan> henninge: is there no test needed that the 'queued' user may access a translationgroup?
[15:10] <henninge> al-maisan: that test is included in the ubuntu tests.
[15:10] <adeuring> noodles775: r10020 of the branch  contains the improved link. Now ready for a re-review :)
[15:10] <al-maisan> henninge:  ubuntu tests?
[15:10] <noodles775> adeuring: great, thanks!
[15:10] <henninge> al-maisan: if you look at the full file, you will see that there is a special test when the target is a sourcepackage in ubuntu
[15:11] <al-maisan> henninge: ah, I see.
[15:11] <al-maisan> henninge: r=me
[15:11] <henninge> al-maisan: that is when translation group is accessed
[15:11] <henninge> al-maisan: Danke!
[15:13] <al-maisan> allenap: sorry, need to finish something else first. Hopefully back to review your stuff soon.
[15:13] <allenap> al-maisan: It's okay, I'm not in a rush. Thanks for letting me know.
[15:17] <al-maisan> :)
[15:29] <noodles775> allenap: wow, that feature is great!
[15:29] <noodles775> allenap: I was wondering whether there was a decision behind *not* displaying the count for anon users?
[15:30] <allenap> noodles775: No deliberate decision, just didn't actually think about it, doh.
[15:30] <allenap> noodles775: I should probably add it!
[15:31] <noodles775> allenap: that'd be great!
[15:31] <allenap> noodles775: I'll make it simply say "This bug affects 27 people".
[15:33] <noodles775> Perfect
[15:46] <bac> hi jpds
[15:46] <jpds> Hey bac.
[15:48] <bac> jpds: since you are becoming so prolific (!) you may want to have a look at https://edge.launchpad.net/lpreview-body .  with it you can do 'bzr send' and it'll help you write a good merge proposal cover letter and get it submitted.  a good side effect is it assigns the whole launchpad team as a reviewer, which may give you much better turn-around.
[15:52] <jpds> bac: Aha, that's neat, thanks for the tip. :)
[15:52] <bac> jpds: oh, and it runs 'make lint' for you too.  that saves me all the time from doing something silly.
[15:59] <jpds> bac: Installed, will use it next time.
[16:03] <bac> jpds: great.  hope it works well for you.
[16:08] <jpds> bac: Can you feed my pending branch to PQM? :)
[16:09] <bac> jpds: i can later.  you may want to see if noodles775 or al-maisan can do it, since they are on-call reviewers today.  what is the URL for the MP?
[16:10] <jpds> https://code.edge.launchpad.net/~jpds/launchpad/fix_50580/+merge/15892
[16:10]  * al-maisan is fixing the devel breakage, sorry
[16:11] <adeuring> al-maisan: ar you still reviewing? If you, could you have a look at https://code.edge.launchpad.net/~adeuring/launchpad/bug-172507/+merge/15925 ? (the real diff isn't 1100 line, but ca 180)
[16:11] <noodles775> rockstar: are you around soon? al-maisan and myself are finishing up, but there are a few requests coming in.
[16:11] <rockstar> noodles775, yea, right after this QA meeting.
[16:11] <al-maisan> adeuring: I am preparing a testfix for devel, later maybe..
[16:11] <noodles775> Great! Thanks rocksar.
[16:12] <adeuring> al-maisan: let me ask rockstar
[16:12] <noodles775> adeuring, jpds: I've put you both in the queue, so rockstar will get to them as soon as he's finished the meeting.
[16:13] <adeuring> noodles775: ok, thanks!
[16:14] <jpds> noodles775: Thanks.
[16:22] <allenap> noodles775: Just thinking. Do you want the link to +affectsmetoo to appear to anon users? So they can click, login, answer?
[16:22] <allenap> noodles775: I've just finished the change to just show a statement without a link!
[16:24] <adiroiban> danilos: even though maybe now we have more privileges, they will not show up in the UI since all TAL conditions are for launchpad.Admin
[16:24] <allenap> noodles775: The context menu only enables the link for logged-in users, so perhaps I'll just follow that.
[16:25] <danilos> adiroiban, heh, but code is right there for people to see :) btw, relevant conditions should be changed as well, no reason to hide them if you ask me
[16:27] <adiroiban> danilos: yep. I will try to solve in in the cleanup branch... as I will need to look at each launchpad.Admin and check if it can be replaced by launchpad.TranslationsAdmin
[16:28] <danilos> adiroiban, it can't be done everywhere, unless you want to get into more of these cleanups :) your effort can be better spent elsewhere than cleaning up the entire tree, not that I'd stop you :)
[16:29] <adiroiban> danilos: hm... then I should fix those problems in this branch ?
[16:30] <danilos> adiroiban, well, no... the point is that launchpad.Admin and launchpad.TranslationsAdmin are not interchangeable permissions
[16:30] <danilos> adiroiban, if there are problems like these (I suspect there are only one or two), we'll fix them as we find them
[16:30] <adiroiban> danilos: ah. ok :)
[16:33] <rockstar> jpds, hi.
[16:34] <jpds> rockstar: Hello.
[16:35] <rockstar> jpds, have you contributed to Launchpad before?
[16:35] <jpds> rockstar: Two branches, both in devel/
[16:35] <rockstar> jpds, okay, so you have signed the contributor agreement then.
[16:36] <jpds> rockstar: I work for Canonical.
[16:36] <rockstar> jpds, oh.
[16:36]  * rockstar hangs head in shame
[16:37]  * jpds hugs rockstar.
[16:37] <rockstar> jpds, ah, you started last month?
[16:37] <jpds> rockstar: Yes.
[16:38] <rockstar> jpds, well welcome aboard!
[16:38] <jpds> Thank you. :)
[16:38] <rockstar> Now I don't feel so bad.  We hire people faster than I can meet people.  :)
[16:39] <rockstar> jpds, have you thought about a way to test this?
[16:40] <jpds> rockstar: I ran: "make run" and viewed the mirrors page at launchpad.dev/ubuntu/+archivemirrors .
[16:40] <rockstar> jpds, well, I mean using automated testing.
[16:40] <jpds> Hmm, not really I'm afraid.
[16:41] <jpds> It's really just colouring in text that's there in black. :)
[16:41] <rockstar> jpds, I'm not sure if it really requires it, but I think it'd be a good benefit.
[16:41] <rockstar> bigjools, ping
[16:41] <bigjools> rockstar: yarp
[16:42] <rockstar> bigjools, https://code.edge.launchpad.net/~jpds/launchpad/fix_50580/+merge/15892
[16:42] <rockstar> bigjools, do you think that this patch needs tests?
[16:43] <bigjools> depends how anal you wanna be
[16:43] <bigjools> the question I ask, is it worth testing
[16:45] <bigjools> it probably is worth a quick test I think, it's pretty easy
[16:45] <rockstar> bigjools, I don't think it requires it, but if it's easy, then why not.
[16:46] <bigjools> yeah, just getTagsByClass and profit
[16:46] <rockstar> jpds, is this something you've done before?
[16:46] <jpds> rockstar: No.
[16:46] <bigjools> rockstar: why don't you write the test for him?
[16:47] <bigjools> we should not block contributions on lack of tests
[16:47] <rockstar> bigjools, yes, this is true.
[16:47] <bigjools> jpds can see the test afterwards and learn :)
[16:49] <jpds> Yeah, I've never written a test case before...
[16:50] <bigjools> jpds: the tests for the web pages are in lib/lp/<project>/stories
[16:50] <bigjools> they use the zope test browser
[16:52] <allenap> al-maisan: If you're still busy, I can see if someone else can have a look at my branch?
[16:52] <al-maisan> allenap: that would be good, I am about to finish the test fix and planned to end the day.
[16:53] <rockstar> bigjools, is there an existing doctest that this might go with?  I know very little about your crazy soyuz code.
[16:54] <bigjools> this is not soyuz code :)
[16:54] <bigjools> it's registry
[16:55] <bigjools> even before we generously donated half our code to them
[16:55] <bigjools> but I can look
[16:56] <bigjools> rockstar: ib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt ?
[16:56] <rockstar> bigjools, thanks.
[16:56] <rockstar> It's distro mirror junk, so I figured you'd know.
[16:56] <noodles775> rockstar: I'm putting one in the queue for a UI review - if you have time later in your day (not urgent).
[16:56] <noodles775> https://code.edge.launchpad.net/~michael.nelson/launchpad/211008-visual-indicator-superseded-pkgs/+merge/15963
[16:56] <rockstar> noodles775, great, thanks.
[16:57] <noodles775> Thank you!
[17:48] <jpds> rockstar: Any luck?
[17:48] <rockstar> jpds, I'm actually on the phone chasing something else right now.
[17:48] <jpds> Ah, OK.
[17:52] <rockstar> jpds, so, the code is good, but I need to write tests, and I wasn't entirely planning to write code much today (being on-call reviewer) so I need to figure out how to work out where to fit that into my day.
[18:41] <jpds> rockstar: OK, right.
[19:12] <rockstar> adeuring, I can't find your branch on +activereviews
[19:12] <adeuring> rockstar: https://code.edge.launchpad.net/~adeuring/launchpad/bug-172507/+merge/15925
[19:12] <rockstar> adeuring, danke
[19:13] <adeuring> rockstar: thanks to _yout_, for reviewing this stuff :)
[19:14] <rockstar> adeuring, holy crap, that's a big SVG...  :)
[19:14] <adeuring> rockstar: well, xml is chatty...
[19:15] <rockstar> adeuring, I'm sure the Ubuntu Bugs team is going to love this.  They were talking about it a lot of UDS.
[19:16] <rockstar> adeuring, I'm also sure that they're going to want this through the API, so eventually you may need to move that to the model/interface
[19:17] <adeuring> rockstar: maybe... But with the API you can simpy filter by the property type
[19:17] <adeuring> so there is no real need, I thnk
[19:17] <adeuring> ...i mean: filter by bugattamnet.type ...
[19:17] <adeuring> argh... bugattachment.type....
[19:20] <rockstar> adeuring, okay.
[19:22]  * rockstar lunches
[19:51] <bac> rockstar: i'm adding a branch to your queue for your post-lunch enjoyment.
[21:13] <abentley> rockstar: I can has reviews?
[21:13] <rockstar> abentley, please add yourself to the queue.
[21:15] <abentley> rockstar: https://code.edge.launchpad.net/~abentley/launchpad/twistedjob-enhancements/+merge/15977 https://code.edge.launchpad.net/~abentley/launchpad/twistedjob/+merge/15980 https://code.edge.launchpad.net/~abentley/launchpad/ampoulejob/+merge/15982 https://code.edge.launchpad.net/~abentley/launchpad/ampoulejob-timeout/+merge/15986
[21:15] <rockstar> abentley, okay, I think you just filled my queue for the rest of the day.
[21:15] <abentley> rockstar: My pleasure.
[21:16] <rockstar> abentley, and I also can't guarantee I'll get to all of them, but I'll do my best.
[21:16] <abentley> rockstar: cool.
[22:58] <jpds> rockstar: getTagsByClass> Does this even exist?
[22:58] <jpds> rockstar: Can't find any reference of it in the code anywhere...
[22:59] <rockstar> jpds, we may not be using it, but it may be part of the zope test browser.
[22:59] <rockstar> ...or BeautifulSoup
[23:00] <jpds> Hmm, Google doesn't seem to know about it.
[23:01] <rockstar> jpds, huh.  Maybe bigjools is on crack.
[23:01] <jpds> rockstar: Well, soyuz... yeah.
[23:02]  * jpds runs.
[23:02] <bigjools> oO
[23:02] <bigjools> find_tags_by_class
[23:02] <bigjools> my bad
[23:03] <jpds> bigjools: Ah cool, thanks.
[23:03] <wgrant> Ah, it's LP-specific.
[23:33] <jpds> rockstar: Tests added: 
[23:34] <jpds> https://code.edge.launchpad.net/~jpds/launchpad/fix_50580/+merge/15892
[23:34] <rockstar> jpds, oh great.  That makes my life easier.
[23:34] <jpds> rockstar: That's always a bonus.