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