| EdwinGrubbs | sinzui: did you already test your query's performance on staging? | 00:25 |
|---|---|---|
| sinzui | No. I have not | 00:25 |
| 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:26 |
| 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:27 |
| lfaraone | EdwinGrubbs: pushed up rev 10009 on lp:~lfaraone/launchpad/bugfix494055, it should fix your concerns. | 00:32 |
| EdwinGrubbs | lfaraone: looking at it now | 00:50 |
| * thumper looks for a reviewer | 01:49 | |
| thumper | EdwinGrubbs: still on? | 01:49 |
| === 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 | ||
| 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:50 |
| EdwinGrubbs | thumper: sorry, my day really ended a while ago. | 01:51 |
| sinzui | thumper: I can do in a few minutes | 01:52 |
| 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:54 |
| sinzui | fab | 01:55 |
| thumper | sinzui: it is there now | 01:55 |
| thumper | sinzui: I'm just popping out to collect the girls from school | 01:57 |
| thumper | sinzui: back shortly | 01:57 |
| 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:00 |
| lfaraone | EdwinGrubbs: cool, thanks. | 02:10 |
| thumper | sinzui: back | 02:11 |
| === thumper is now known as eric-the-viking | ||
| === eric-the-viking is now known as thumper | ||
| thumper | sinzui: are you checking it out? | 02:39 |
| thumper | sinzui: I have to head off in about 5-10 minutes | 02:39 |
| 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 | 02:40 |
| 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 | 07:31 |
| === 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 | ||
| 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:26 |
| 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:27 |
| adiroiban | danilos: ok | 09:28 |
| danilos | adiroiban, ok, I am landing your branch as well | 09:40 |
| adiroiban | danilos: thanks. still reading the review :) | 09:40 |
| danilos | adiroiban, heh, right :) | 09:41 |
| adiroiban | danilos: how is the inheritance working for classes from security.py ? I think's I'm missing a python mechanism | 10:12 |
| adiroiban | checkAuthenticated return True | 10:13 |
| adiroiban | and if it returns None, it will call super ? | 10:13 |
| 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:14 |
| 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:15 |
| 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:16 |
| 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:17 |
| adiroiban | and I wonder why it is not AdminDistributionTranslations(AuthorizationBase): | 10:18 |
| 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:19 |
| danilos | adiroiban, it's just a trick which involves knowledge of python OO model :) | 10:21 |
| 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:37 |
| 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 |
| === 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 | ||
| adeuring | noodles775: yeah... but please have a look at the "code" first ;) | 10:38 |
| noodles775 | ah, ok, it's like that then :-) | 10:39 |
| noodles775 | No worries. | 10:39 |
| noodles775 | I see, literally two lines! It's mine I tell you! | 10:40 |
| 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:52 |
| al-maisan | bigjools: will do. | 10:53 |
| noodles775 | ta, I'll do adeuring's branch. | 10:53 |
| bigjools | thanks guys | 10:53 |
| 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:54 |
| al-maisan | bigjools: no problem -- just let me know when it's done. | 10:55 |
| bigjools | diff will regenerate itself | 10:55 |
| bigjools | al-maisan: https://code.edge.launchpad.net/~julian-edwards/launchpad/die-buildqueue-die--bug-492632/+merge/15927 | 11:01 |
| al-maisan | bigjools: thanks! | 11:02 |
| === 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 | ||
| bigjools | grrrr the person chooser on the MPs gives you no chance to review what you clicked on | 11:03 |
| bigjools | BjornT_: can you review the above MP for r-c please | 11:04 |
| === 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 | ||
| al-maisan | bigjools: r=me | 11:13 |
| bigjools | ta | 11:15 |
| 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:29 |
| adeuring | noodles775: right, just gor remonder from deryck :) I'm adding that now | 11:30 |
| noodles775 | Great. | 11:30 |
| === 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 | ||
| 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:31 |
| noodles775 | danilos: yeah, I'll take it (I think al-maisan is on lunch) | 11:32 |
| danilos | noodles775, cool, thanks | 11:32 |
| === 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 | ||
| 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:38 |
| 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:39 |
| 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:40 |
| 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:41 |
| 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:42 |
| danilos | adiroiban, at least 'color: grey' should do for now, or a light background | 11:43 |
| 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:44 |
| 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:45 |
| 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:46 |
| 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:47 |
| 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:48 |
| 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:49 |
| 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:50 |
| 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:51 |
| 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:52 |
| adiroiban | uh... color ? | 11:53 |
| adiroiban | colorcode | 11:53 |
| adiroiban | #DCDCDC | 11:54 |
| * noodles775 looks in style-3.0.css | 11:54 | |
| noodles775 | #747474 is what's being used for a disabled tab, but it might be too dark. See what you think. | 11:55 |
| adiroiban | nope. to dark | 11:56 |
| adiroiban | that was my starting point | 11:56 |
| adiroiban | let me try some colors and show you some screenshots | 11:59 |
| 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:01 |
| adiroiban | danilos: should I rename the other xx-rosetta-pofile stories to xx-pofile? | 12:06 |
| danilos | adiroiban, sure | 12:07 |
| 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:08 |
| adiroiban | danilos: or they can create name conflicts? | 12:09 |
| danilos | adiroiban, yeah, that too | 12:09 |
| adiroiban | with stories from other products? | 12:09 |
| 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:10 |
| beuno | noodles775, still need that peak? | 12:31 |
| === henninge is now known as henninge-lunch | ||
| noodles775 | beuno: yeah, if you wouldn't mind, then I can finish it and put an MP together :-) Thanks! | 12:32 |
| beuno | noodles775, where does "newer version" link? | 12:36 |
| noodles775 | beuno: https://launchpad.dev/ubuntu/warty/+source/iceweasel/1.1 | 12:37 |
| 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:38 |
| noodles775 | beuno: Erm, I don't think so, one tick... | 12:39 |
| bigjools | BjornT_: I dunno without looking | 12:39 |
| 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:40 |
| 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:41 |
| beuno | noodles775, in light of that, what do you think about making it less prominent? | 12:42 |
| noodles775 | beuno: sure (I just checked, anything published in primary, partner or debug is included). | 12:43 |
| 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:44 |
| 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:45 |
| * beuno thinks | 12:46 | |
| bigjools | BjornT_: heh, there's a few without the >>> | 12:46 |
| bigjools | that's an old old test | 12:46 |
| 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:47 |
| beuno | noodles775, maybe less verbose? "(i) _Newer version_ available" | 12:49 |
| 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:50 |
| 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:51 |
| 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:54 |
| BjornT_ | salgado: ok | 12:55 |
| === mrevell is now known as mrevell-lunch | ||
| 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:00 |
| 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:01 |
| 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:02 |
| salgado | BjornT_, canonical/widgets/templates/inline-picker.pt | 13:04 |
| BjornT_ | salgado: not vocabulary-picker.js? | 13:06 |
| salgado | BjornT_, could be; that's not quite clear to me | 13:09 |
| BjornT_ | salgado: yeah. i'll add a comment to the MP. i have some thoughts on the API | 13:11 |
| 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:13 |
| BjornT_ | bigjools: yeah, it's better to be on the safe side | 13:14 |
| bigjools | I agree | 13:15 |
| salgado | BjornT_, did you forget about that comment to the MP? | 13:35 |
| 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:39 |
| === mrevell-lunch is now known as mrevell | ||
| danilos | adiroiban, why's that? | 13:48 |
| adiroiban | danilos: http://paste.ubuntu.com/338672/ | 13:49 |
| adiroiban | danilos: I assume that as an UTC i only got an launchpad.TranslationsAdmin "token" | 13:50 |
| 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:51 |
| 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:53 |
| danilos | adiroiban, cool, let me get it here | 13:54 |
| adiroiban | danilos: here is a "clean" error http://paste.ubuntu.com/338675/ | 13:55 |
| adiroiban | danilos: you get that when trying to save a template as UTC | 13:56 |
| 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:01 |
| adiroiban | danilos: thanks. I was looking only at translations/browser/configure.zcml | 14:04 |
| danilos | adiroiban, np | 14:07 |
| 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:08 |
| * danilos switches locations | 14:12 | |
| === 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 | ||
| 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:27 | |
| 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 |
| === 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 | ||
| henninge | noodles775: ok, I was just curious, und außerdem ^ | 14:28 |
| henninge | al-maisan: Danke schön! | 14:28 |
| al-maisan | henninge: Bitte, nichts zu danken :) | 14:29 |
| 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:40 |
| === 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 | ||
| allenap | noodles775: Cool, thanks :) | 14:41 |
| noodles775 | allenap: np. I'll take a look in 15m or so. | 14:41 |
| 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:42 |
| 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:43 |
| henninge | allenap: do we still split js and code? | 14:44 |
| 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:45 |
| 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:46 |
| 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:47 |
| henninge | allenap: yup, next reviewers meeting on the 6th. | 14:48 |
| 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:50 |
| henninge | allenap: https://dev.launchpad.net/ReviewerMeetingAgenda | 14:51 |
| allenap | henninge: Fantastic :) | 14:51 |
| adiroiban | danilos: hi. I pushed the latest change, and after running bin/test -vvc -m translations --layer=PageTestLayer , everhing seems fine. | 14:55 |
| danilos | adiroiban, cool, sounds good, I'll take a look at it asap | 15:00 |
| === salgado is now known as salgado-lunch | ||
| adiroiban | danilos: there's no hurry :) | 15:01 |
| === matsubara is now known as matsubara-lunch | ||
| al-maisan | henninge: is there no test needed that the 'queued' user may access a translationgroup? | 15:09 |
| 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:10 |
| 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:11 |
| === 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 | ||
| 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:13 |
| al-maisan | :) | 15:17 |
| 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:29 |
| allenap | noodles775: No deliberate decision, just didn't actually think about it, doh. | 15:30 |
| allenap | noodles775: I should probably add it! | 15:30 |
| noodles775 | allenap: that'd be great! | 15:31 |
| allenap | noodles775: I'll make it simply say "This bug affects 27 people". | 15:31 |
| noodles775 | Perfect | 15:33 |
| bac | hi jpds | 15:46 |
| jpds | Hey bac. | 15:46 |
| 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:48 |
| 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:52 |
| === matsubara-lunch is now known as matsubara | ||
| === beuno is now known as beuno-lunch | ||
| jpds | bac: Installed, will use it next time. | 15:59 |
| bac | jpds: great. hope it works well for you. | 16:03 |
| jpds | bac: Can you feed my pending branch to PQM? :) | 16:08 |
| 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:09 |
| jpds | https://code.edge.launchpad.net/~jpds/launchpad/fix_50580/+merge/15892 | 16:10 |
| * al-maisan is fixing the devel breakage, sorry | 16:10 | |
| 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:11 |
| adeuring | al-maisan: let me ask rockstar | 16:12 |
| === 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 | ||
| 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:12 |
| adeuring | noodles775: ok, thanks! | 16:13 |
| jpds | noodles775: Thanks. | 16:14 |
| 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:22 |
| 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:24 |
| 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:25 |
| 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:27 |
| 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:28 |
| adiroiban | danilos: hm... then I should fix those problems in this branch ? | 16:29 |
| 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:30 |
| === 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 | ||
| rockstar | jpds, hi. | 16:33 |
| jpds | rockstar: Hello. | 16:34 |
| 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:35 |
| jpds | rockstar: I work for Canonical. | 16:36 |
| rockstar | jpds, oh. | 16:36 |
| * rockstar hangs head in shame | 16:36 | |
| * jpds hugs rockstar. | 16:37 | |
| === salgado-lunch is now known as salgado | ||
| rockstar | jpds, ah, you started last month? | 16:37 |
| jpds | rockstar: Yes. | 16:37 |
| 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:38 |
| === danilos is now known as danilo-bblot | ||
| rockstar | jpds, have you thought about a way to test this? | 16:39 |
| 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:40 |
| 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:41 |
| 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:42 |
| bigjools | depends how anal you wanna be | 16:43 |
| bigjools | the question I ask, is it worth testing | 16:43 |
| === beuno-lunch is now known as beuno | ||
| 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:45 |
| 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:46 |
| 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:47 |
| jpds | Yeah, I've never written a test case before... | 16:49 |
| bigjools | jpds: the tests for the web pages are in lib/lp/<project>/stories | 16:50 |
| bigjools | they use the zope test browser | 16:50 |
| 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:52 |
| rockstar | bigjools, is there an existing doctest that this might go with? I know very little about your crazy soyuz code. | 16:53 |
| bigjools | this is not soyuz code :) | 16:54 |
| bigjools | it's registry | 16:54 |
| bigjools | even before we generously donated half our code to them | 16:55 |
| bigjools | but I can look | 16:55 |
| 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:56 |
| === 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 | ||
| noodles775 | Thank you! | 16:57 |
| 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:48 |
| 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. | 17:52 |
| === deryck is now known as deryck[lunch] | ||
| jpds | rockstar: OK, right. | 18:41 |
| === 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 | ||
| 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:12 |
| adeuring | rockstar: thanks to _yout_, for reviewing this stuff :) | 19:13 |
| rockstar | adeuring, holy crap, that's a big SVG... :) | 19:14 |
| adeuring | rockstar: well, xml is chatty... | 19:14 |
| rockstar | adeuring, I'm sure the Ubuntu Bugs team is going to love this. They were talking about it a lot of UDS. | 19:15 |
| 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:16 |
| 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:17 |
| rockstar | adeuring, okay. | 19:20 |
| * rockstar lunches | 19:22 | |
| === deryck[lunch] is now known as deryck | ||
| bac | rockstar: i'm adding a branch to your queue for your post-lunch enjoyment. | 19:51 |
| === 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 | ||
| abentley | rockstar: I can has reviews? | 21:13 |
| rockstar | abentley, please add yourself to the queue. | 21:13 |
| === 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 | ||
| 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:15 |
| 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. | 21:16 |
| jpds | rockstar: getTagsByClass> Does this even exist? | 22:58 |
| jpds | rockstar: Can't find any reference of it in the code anywhere... | 22:58 |
| rockstar | jpds, we may not be using it, but it may be part of the zope test browser. | 22:59 |
| rockstar | ...or BeautifulSoup | 22:59 |
| jpds | Hmm, Google doesn't seem to know about it. | 23:00 |
| rockstar | jpds, huh. Maybe bigjools is on crack. | 23:01 |
| jpds | rockstar: Well, soyuz... yeah. | 23:01 |
| * jpds runs. | 23:02 | |
| bigjools | oO | 23:02 |
| bigjools | find_tags_by_class | 23:02 |
| bigjools | my bad | 23:02 |
| jpds | bigjools: Ah cool, thanks. | 23:03 |
| wgrant | Ah, it's LP-specific. | 23:03 |
| jpds | rockstar: Tests added: | 23:33 |
| 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. | 23:34 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
