| === adiroiba1 is now known as adiroiban | ||
| === Ursinha is now known as Ursinha-afk | ||
| === danilo_ is now known as danilos | ||
| === StevenK_ is now known as StevenK | ||
| === al-maisan changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [adiroiban(bug-492375 pre-implementation), al-maisan(http://tinyurl.com/y8joo24)] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === jtv changed the topic of #launchpad-reviews to: on call: jtv || reviewing: - || queue [adiroiban(bug-492375 pre-implementation), al-maisan(http://tinyurl.com/y8joo24)] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === jtv changed the topic of #launchpad-reviews to: on call: jtv || reviewing: adiroiban || queue [al-maisan(http://tinyurl.com/y8joo24)] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === allenap changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: adiroiban, - || queue [al-maisan(http://tinyurl.com/y8joo24)] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | adiroiban: you're trying to get a pre-implementation call for your branch, right? | 09:14 |
|---|---|---|
| adiroiban | jtv: yep. but there is no hurry | 09:15 |
| adiroiban | i just want to know if it't ok to augment a model in the view in that way | 09:16 |
| gmb | jtv, allenap: RC branch for your perusal, if you please: https://code.edge.launchpad.net/~gmb/launchpad/ubuntu-filebug-dupefinder-bug-497096/+merge/16230 | 09:16 |
| jtv | adiroiban: oops, gmb's takes precedence. I'll let yours lie for the moment, then | 09:17 |
| adiroiban | jtv: sure :) | 09:17 |
| allenap | jtv: I can take it. | 09:17 |
| jtv | allenap: oh, that would solve it too. :-) | 09:17 |
| jtv | hi btw | 09:17 |
| === allenap changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: adiroiban, gmb || queue [al-maisan(http://tinyurl.com/y8joo24)] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | Hi jtv :) | 09:18 |
| allenap | al-maisan: Is your branch an RC candidate? | 09:18 |
| jtv | adiroiban: so back to yours... I'll browse through your diffs a bit first | 09:18 |
| adiroiban | jtv: look at the view class | 09:18 |
| al-maisan | allenap: no, it's not | 09:18 |
| al-maisan | allenap: .. and good morning :) | 09:19 |
| adiroiban | jtv: i would like to tag preferred langauages so that it makes it easy and nice to presented them different in the template | 09:19 |
| allenap | al-maisan: Morning :) | 09:19 |
| jtv | adiroiban: I'm comparing to productseries to get an idea of what danilo is saying | 09:20 |
| adiroiban | jtv: I talked with danilo and we agree to do the filtering using css class and javascript | 09:22 |
| adiroiban | jtv: the current branch is not ready yet. I just create a filter on a single page | 09:23 |
| adiroiban | jtv: here is the nasty code http://bazaar.launchpad.net/~adiroiban/launchpad/bug-492375/revision/10055#lib/lp/translations/browser/distroseries.py | 09:24 |
| jtv | adiroiban: thanks | 09:24 |
| jtv | oh, that's the same page I was already looking at :) | 09:25 |
| adiroiban | :) | 09:26 |
| danilos | adiroiban, btw, I think your branch already fix that bug 347... (allowing translation group owners to administer langpacks) | 09:26 |
| jtv | hi danilo :) | 09:26 |
| danilos | jtv, hi | 09:26 |
| adiroiban | danilos: maybe :) i have no idea how lang-packs are handled | 09:26 |
| jtv | I think we have a "link" formatter for languages... makes me wonder if the CSS class for preferred languages should be in there, and the default "seen/unseen" choice for non-preferred ones set in local CSS | 09:28 |
| adiroiban | danilos: i just commented on that bug and assigned so that I can look into it | 09:28 |
| danilos | adiroiban, it should be pretty simple to test, log in as an ubuntu translation group owner but without any admin or rosetta-experts privileges to launchpad.dev and go to translations.launchpad.dev/ubuntu/hoary/+language-packs and see if the form shows up and you can edit it | 09:28 |
| allenap | gmb: Should lines 20 to 33 also be moving? | 09:28 |
| danilos | jtv, it should, I'd say! | 09:28 |
| danilos | jtv, I mean, it's a great idea :) | 09:29 |
| danilos | adiroiban, yep, thanks | 09:29 |
| jtv | oh, probably in js, not css—so that you get all languages when js is disabled | 09:29 |
| jtv | (barring cases where we have a clickthrough to a full list I guess) | 09:29 |
| gmb | allenap: Eh... Hmm. Well, it passes the tests ;). But no, probably not. I'll move those back. | 09:29 |
| danilos | adiroiban, if you find it's fixed by your previous branch, please do assign yourself to it... if it's not, it can be because we are not using the same privilege on distroseries, and then it's still only a simple fix away | 09:29 |
| gmb | allenap: Although that gets a bit knotty then... | 09:29 |
| adiroiban | danilos: yep. let me start firefox | 09:30 |
| danilos | jtv, well, we only set class "preferred" on preferred languages, but language formatter is not very useful here since we are linking to ProductSeriesLanguage or DistroSeriesLanguage (or in the future, ProjectLanguage or SourcePackageLanguage) | 09:30 |
| adiroiban | it looks like in webkit based browser I can not see some sprites | 09:30 |
| gmb | allenap: No, I'll move it back, because otherwise it will have nasty effects on the other +filebug views when extra data get passed. | 09:30 |
| jtv | danilos: facepalm :) | 09:30 |
| gmb | allenap: Nice catch. | 09:31 |
| danilos | adiroiban, yeah, I've seen that too, I think we have an existing bug on those | 09:31 |
| jtv | we have a problem with konqueror with <a> tags that have no text in them... | 09:32 |
| adiroiban | danilos: checking in firefox... no new edit stuff. I will look into that as by now I should know how to fix it | 09:32 |
| jtv | there's a little workaround for the konqueror problem though | 09:32 |
| adiroiban | share it :) | 09:33 |
| adiroiban | i'm using chromium and epiphany and I can not see the import queue entry edit icons | 09:33 |
| * jtv digs up lazr-js fix | 09:33 | |
| adiroiban | danilos: as soon as I finish with the current branches I will start hunting the lang-pack bug. Right now I'm looking at import queue permission | 09:35 |
| danilos | adiroiban, I am not going to push you at all, it's not like you need pushing :) so, just take your time :)) | 09:35 |
| adiroiban | danilos: :) | 09:36 |
| adiroiban | i'm still learning the LP security mechanism | 09:36 |
| allenap | gmb: I have broken your merge proposal. | 09:36 |
| gmb | allenap: Fail. | 09:37 |
| gmb | allenap: Just do the second review that's listed for you; it should DTRT. | 09:37 |
| allenap | gmb: No, I mean it's OOPSing all the time when I go to the page. | 09:38 |
| jtv | adiroiban: okay, it's nasty: if you have a link with just an icon in it (nothing else), konqueror sees it as empty and so doesn't render it. You can work around it in CSS. To :link and :visited pseudoclasses for the link, you add content: "" | 09:38 |
| allenap | gmb: https://lp-oops.canonical.com/oops.py/?oopsid=OOPS-1446EB307 | 09:38 |
| adiroiban | how long does it take to run a full test on ec2? On an 2G AMD, running test -m translations it took me 200minutes | 09:38 |
| jtv | adiroiban: that's pretty long... but a full ec2 test is going to take longer | 09:38 |
| jtv | adiroiban: but | 09:39 |
| allenap | gmb: I'm filing a bug for it. | 09:39 |
| allenap | gmb: In any case, r=me. | 09:39 |
| jtv | I'm pretty sure you can fire off an ec2 image with your own arguments for bin/test | 09:39 |
| gmb | allenap: Ta. I'll have an incremental diff for you in a second... | 09:39 |
| allenap | gmb: Cool. | 09:40 |
| adiroiban | jtv: np. I was just courious | 09:40 |
| jtv | adiroiban: the old ec2test command would just pass on any extra args | 09:40 |
| adiroiban | jtv: so getting back at my branch (sorry for going offtopic) | 09:41 |
| gmb | allenap: Inc diff posted to https://code.edge.launchpad.net/~gmb/launchpad/ubuntu-filebug-dupefinder-bug-497096/+merge/16230 | 09:41 |
| * gmb asks Santa for permalinks to MP comments. | 09:42 | |
| allenap | gmb: That page is OOPSing for me :) I'll wait for the email. | 09:42 |
| gmb | Hah. | 09:43 |
| jtv | adiroiban: yes... afaics, if js is turned off, your branch shows only the preferred languages. Don't we want all languages in that case? | 09:43 |
| gmb | allenap: http://pastebin.ubuntu.com/342580/ if you want it quicker ;) | 09:43 |
| adiroiban | jtv: ok. sure | 09:43 |
| adiroiban | jtv: i was worried about the usage of class DistroSeriesLanguageTagged: | 09:44 |
| jtv | adiroiban: yes, I do wonder whether that's really needed | 09:44 |
| allenap | gmb: I got it, thanks. Looks good, r=me. | 09:45 |
| adiroiban | jtv: adding it as an attribute to SeriesLanguage model is also strange | 09:45 |
| gmb | allenap: Thanks. | 09:45 |
| adiroiban | as it's only a view attribute | 09:45 |
| adiroiban | and the model should not care about it | 09:45 |
| allenap | gmb: I'll try and reply to the mp via email to approve it too. | 09:45 |
| gmb | Thanks. | 09:46 |
| jtv | adiroiban: very true | 09:47 |
| adiroiban | jtv: this is my dilemma | 09:47 |
| adiroiban | jtv: i could just put that logic in the template... but it would make the template look bad | 09:48 |
| adiroiban | s/bad/ugly/ | 09:48 |
| * jtv wonders if factoring out a SeriesLanguageView would help | 09:48 | |
| adiroiban | jtv: SeriesLanguageView or ( DistroSeriesView and ProductSeriesView) ? | 09:51 |
| jtv | adiroiban: a replacement for ProductSeriesLanguageView & DistroSeriesLanguageView | 09:52 |
| jtv | adiroiban: they're practically identical, structurally | 09:53 |
| adiroiban | jtv: yes... but I don't know how this can solve the langauge tagging problem | 09:54 |
| jtv | but you'd have to render the table rows differently to get to that view | 09:54 |
| jtv | well, you'd have a single place to tag a <tr> as being for a preferred language or not | 09:54 |
| adiroiban | jtv: yes. but my problem is how to tag it in the view | 09:55 |
| jtv | maybe I'm thinking too much about the cleanup part :) | 09:55 |
| allenap | gmb: I can't update that mp by email either (bug 497338). | 09:55 |
| mup | Bug #497338: OOPS on merge proposal page <oops> <Launchpad Bazaar Integration:New> <https://launchpad.net/bugs/497338> | 09:55 |
| adiroiban | jtv: i can do the cleanup... but I still don't know how to tag a preferred langauge in the view | 09:55 |
| danilos | jtv, adiroiban: do note that when you are going through a list of those, they are read from an attribute on the model class, but the view class extends them with preferred languages; that's the right place to add "preferred" decorators on them I guess | 09:55 |
| adiroiban | jtv: other than the proposed solution | 09:55 |
| adiroiban | danilos: I think that's what I'm doing now... or not? | 09:56 |
| danilos | adiroiban, I don't know :) | 09:57 |
| adiroiban | danilos: ok :) | 09:57 |
| jtv | adiroiban: I don't think anyone is saying the basic low-level approach is wrong | 09:57 |
| jtv | but we're talking about where to put it | 09:57 |
| jtv | since we already have view classes for {product,distro}serieslanguage | 09:58 |
| gmb | allenap: What did you do? Honestly... Okay, no worries, I'll add a note to the effect that you broke everything. | 09:58 |
| allenap | gmb: Cool :) | 09:59 |
| jtv | adiroiban: It may start to matter as the code deals with more special cases: not logged in, no preferences set, no JS enabled. Don't want to duplicate that code. | 09:59 |
| adiroiban | jtv: I can add an interface/baseclass... but I don't know how to avoid the usage of SeriesLanguageTagged | 09:59 |
| adiroiban | jtv: or is ok to go with a SeriesLanguageTagged class? | 10:00 |
| danilos | adiroiban, jtv: I'd say just put it in the model for now, since there's a lot more refactoring possible there anyway | 10:00 |
| === allenap changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: adiroiban, al-maisan || queue [] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jtv | danilos: maybe an "is_preferred"? | 10:01 |
| jtv | hmmm that still leaves the messy stuff in the tal | 10:01 |
| adiroiban | danilos: putting it the model will also imply changing the permission | 10:01 |
| danilos | jtv, yeah, it's just a place to hold the value that will be passed in from the construction in the view | 10:02 |
| danilos | adiroiban, what permission would need to change? | 10:02 |
| adiroiban | for accessing the is_preferred | 10:02 |
| adiroiban | as it would be written in the view | 10:02 |
| danilos | adiroiban, well, of course, you'd have to allow it, but you can actually pass it in from the constructor/and or (D|P)SLSet instead | 10:03 |
| adiroiban | danilos: I don't know... I don't feel we need to have that attribute in the model | 10:03 |
| jtv | adi has a point... it gets ugly | 10:04 |
| adiroiban | it is something binded to the user configurations | 10:04 |
| danilos | adiroiban, well, you've got two options then: initialise a view for each of the series languages (even uglier, if you ask me: that's what we are doing for +translate page and it's very bad) or... | 10:04 |
| jtv | now, I'm very very rusty and vague on the zope part of this, but if we can render the table rows from a separate view, then that view seems the ideal place for this—and we have the two near-identical *SeriesLanguageViews already | 10:05 |
| danilos | adiroiban, or create a new decorated SeriesLanguage class which wraps (Product|Distro)SeriesLanguage and contains such things | 10:05 |
| allenap | al-maisan_: Line 332, does getPublishedSources() sort the results? | 10:05 |
| al-maisan_ | allenap: just a minute please | 10:05 |
| danilos | jtv, yes, we can do that in zope | 10:05 |
| jtv | but not nice? | 10:05 |
| adiroiban | danilos: that's what I'm doing now... a new class that wraps | 10:06 |
| danilos | jtv, that's close to what I proposed above, except that it would again be a view | 10:06 |
| adiroiban | danilos: or at least... I think I'm doing it :) | 10:06 |
| jtv | adi: yes, you are... and I must admit it's not a lot of overhead | 10:07 |
| danilos | adiroiban, the problem with that approach is that it gets ugly faster... DSL and PSL should implement the same interface, and then you could have a wrapper class/row view for that interface | 10:07 |
| adiroiban | danilos: i will start with implementing a single interface | 10:07 |
| adiroiban | and the do the wrapper | 10:07 |
| adiroiban | for a single serieslangauge | 10:07 |
| danilos | adiroiban, jtv: and, we should have a shared interface for something that IHasSeriesLanguageObjects which allows you to render these listings, including marking languages as preferred and similar | 10:08 |
| jtv | that does get to be more work... | 10:08 |
| al-maisan_ | allenap: yes, it does. | 10:08 |
| danilos | with that, I don't see why we'd need a wrapper interface anyway, so... :) | 10:08 |
| allenap | al-maisan_: Cool. | 10:08 |
| danilos | jtv, indeed | 10:08 |
| jtv | for the DSL/PSL views we don't even need two separate classes AFAICSC, but that's another matter | 10:09 |
| danilos | jtv, exactly, the views should be registered on the interface they share | 10:09 |
| adiroiban | ok. now it's getting messy :D | 10:09 |
| danilos | jtv, that interface should support ProjectLanguage and SourcePackageLanguage in the (close) future | 10:09 |
| === jpds changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: adiroiban, al-maisan || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| danilos | adiroiban, heh, naah... | 10:10 |
| danilos | adiroiban, jtv: that's why I say let's get this into a model for now, and if we want to clean it up, let's start top-down | 10:11 |
| adiroiban | jtv, danilos: the plan: implement the wraper as it is now. and then look look how all these views can be refactored | 10:11 |
| danilos | adiroiban, jtv: i.e. first step is to make PSL/DSL use a shared interface (even if that interface includes is_preferred, that's ok for now) | 10:11 |
| adiroiban | like you said... it is to much for now | 10:11 |
| adiroiban | so we need to break the work | 10:12 |
| danilos | adiroiban, jtv: next step would be providing a SLCollection interface to be implemented by ProductSeries, DistroSeries,... | 10:12 |
| danilos | adiroiban, exactly, so I'd keep it as simple as possible for now and put it in the model :) if you also refactor PSL/DSL to share the interface and use the same view, that's even better :) | 10:12 |
| adiroiban | danilos: but I still need to hold the css_class as an attribute | 10:13 |
| danilos | adiroiban, or have an ugly TAL :) perhaps tal:define is not that bad in here because it will simplify TAL and let you get on with the actual feature without being too bad ;) | 10:13 |
| adiroiban | so instead of is_preferred, i need to have css_class | 10:13 |
| danilos | adiroiban, yep, see above | 10:13 |
| adiroiban | right now I don't see to much sense in putting in on the model. it should be in the view or the template | 10:15 |
| jtv | we could also have it all in JS, I guess | 10:15 |
| adiroiban | jtv: yes :) | 10:16 |
| jtv | we'd be giving the JS a list of preferred languages | 10:16 |
| adiroiban | jtv: maybe that would be the "cleanest" approach for now | 10:17 |
| danilos | jtv, that's an option as well | 10:17 |
| jtv | well the dirty little skeleton in that can of worms is how to make the tal pass the list to the js | 10:17 |
| allenap | al-maisan_: getChangesFilesForSources() seems to now only be used in lib/lp/soyuz/adapters/archivesourcepublication.py. Is there any way to change that to use your new query? | 10:17 |
| jtv | ...if you don't mind me mixing a few metaphors | 10:18 |
| adiroiban | jtv: just put it in a json/javascript structure | 10:18 |
| jtv | it's not a big skeleton :) | 10:18 |
| al-maisan_ | allenap: I am not sure, that method is selecting and pre-joining a lot more data and that probably makes sense for the web UI | 10:20 |
| jtv | adiroiban: an added advantage (I think) is that the "full view" stays the natural consequence when JS is disabled | 10:21 |
| jtv | and afaics there are no changes when reusing between different "serieslanguage" types | 10:22 |
| allenap | al-maisan_: Okay. Do you think you could comment in getChangesFilesForSources() that getChangesFileLFA() contains a potentially more efficient query, depending on what you need out of it. | 10:22 |
| adiroiban | jtv: http://bazaar.launchpad.net/~adiroiban/launchpad/bug-359180-event-key/revision/8787#lib/lp/translations/templates/pofile-translate.pt line 288 | 10:22 |
| al-maisan_ | allenap: sure, will do that. | 10:23 |
| adiroiban | is a javascript that holds a tal loop | 10:23 |
| adiroiban | in that way I can write the prefered langauges in a js array | 10:23 |
| jtv | danilos, didn't you also do something like what adiroiban just pointed to for PSL? | 10:24 |
| danilos | jtv, btw, doing it in JS is going to make some very ugly TAL | 10:24 |
| allenap | al-maisan_: There aren't any unit tests for getChangesFileLFA(). I can see that changes_file_url is tested in a few places, but it's not quite the same thing. Unless there's a good reason not to, could you add this? | 10:25 |
| al-maisan_ | allenap: good point, I'll add unit tests for getChangesFileLFA(). | 10:25 |
| danilos | jtv, /me looks | 10:25 |
| allenap | al-maisan_: Thanks. | 10:26 |
| adiroiban | danilos: <tal:block tal:replace="structure string:<script type='text/javascript'>" /> | 10:26 |
| adiroiban | to trick tal thinking that you are still in HTML/XHTML land | 10:26 |
| jtv | danilos: actually, not much like that bit. :) What I meant was, each PSL row gets a CSS class with the language code in it | 10:27 |
| danilos | adiroiban, the problem is that you want to get preferred languages from the view using TAL and put that into JS | 10:27 |
| jtv | a trick for that is to embed a single js function call in the tal that passes the desired arguments | 10:28 |
| jtv | really minimizes the tal impact | 10:28 |
| adiroiban | jtv: function... or just an array | 10:28 |
| jtv | ...or an array. :) | 10:28 |
| danilos | adiroiban, jtv: right, and if it's not clear yet, I find that just doing it in the model is much, much nicer | 10:28 |
| danilos | adiroiban, jtv: you can argue sanity of that any day, but I am going to argue sanity of this kind of JS back every time, and I'll still feel like I am right :) | 10:29 |
| adiroiban | danilos: ok. then I will add a "css_class" in the model? | 10:29 |
| adiroiban | :D | 10:29 |
| danilos | adiroiban, no, add is_preferred to the model | 10:29 |
| adiroiban | and how can i convert the is_preferred to a css class? | 10:30 |
| danilos | adiroiban, and in TAL do something like <tal:preferred condition="lang/is_preferred" define="css_class preferred" /> or something like that | 10:30 |
| danilos | adiroiban, using tal:condition | 10:30 |
| * jtv stops trying to interfere :-) | 10:30 | |
| adiroiban | danilos: ok. let me try and see what I can get | 10:31 |
| danilos | adiroiban, alternative is to not add anything to the model, and use the similar tal:condition to define CSS class using a view model and something like tal:condition="python:langstats.language in view.preferredlanguages" | 10:32 |
| danilos | adiroiban, with above, you won't even have to modify the model, just expose preferredlanguages in the view | 10:32 |
| adiroiban | danilos: but I will have to use define="global css_class string:preferred" | 10:33 |
| adiroiban | or otherwise there will be a big duplicate code for the row | 10:33 |
| adiroiban | is this ok? | 10:33 |
| adiroiban | to use global? | 10:33 |
| danilos | adiroiban, yes, don't tell me that's uglier TAL than for inserting JS :) | 10:33 |
| danilos | adiroiban, yeah, it's ok to use a global but then name it something more unique just in case (i.e. preferred_language_css_class) | 10:34 |
| adiroiban | danilos: nope | 10:34 |
| jtv | isn't it going to break the matching def for the next row | 10:34 |
| jtv | though? | 10:34 |
| danilos | jtv, what do you mean? we'll want to define it to an empty string otherwise, that's for sure | 10:34 |
| adiroiban | danilos , jtv : thanks! I will go with TAL only | 10:35 |
| adiroiban | python and global | 10:35 |
| jtv | danilos: don't know what global does there, so wondering whether anything will break if you re-define the same name later | 10:35 |
| danilos | adiroiban, instead of doing an in like I suggested above, it's probably marginally better to have a method on the view isPreferredLanguage and call that instead | 10:35 |
| adiroiban | danilos: ok | 10:36 |
| danilos | jtv, oh, I am sure that'll be fine | 10:36 |
| adiroiban | may the TAL be with me | 10:36 |
| danilos | adiroiban, you'll still have to call the method with python:view.isPreferredLanguage(langstat.language) but I think that's better | 10:36 |
| danilos | adiroiban, :) | 10:36 |
| adiroiban | yep | 10:36 |
| adiroiban | ok. I think it's enough for now | 10:37 |
| jtv | adiroiban: and while we're complicating your life, don't forget the cases where there are no preferred languages. :-) | 10:37 |
| adiroiban | jtv: :) | 10:37 |
| adiroiban | jtv, danilos . that should be all for the pre-implementation call. Thanks! | 10:41 |
| jtv | adiroiban: good luck, thanks for working on this! | 10:41 |
| === jtv changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: -, al-maisan || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === allenap changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: -, - || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === allenap changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: -, <release-qa> || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| jpds | https://code.edge.launchpad.net/~jpds/launchpad/fix_450262/+merge/16203 ← all set for review. :) | 11:12 |
| jtv | jpds: shouldn't you follow that one up with bac? | 11:15 |
| jpds | jtv: Hmm, yeah, I thought it could do with more eyes too. | 11:15 |
| jtv | jpds: makes sense, but it also confuses the situation a bit... like asking your dad for a cookie because your mom already said no. :-) | 11:16 |
| jpds | jtv: But I asked for a biscuit! | 11:17 |
| jtv | jpds: I see you have worked on this strategy | 11:17 |
| jpds | jtv: I'll wait for bac. | 11:39 |
| jtv | jpds: IIRC his review said he'd review the updated MP, right? He should be around soon, if not already. | 11:39 |
| maxb | If you don't mind a slightly different kind of review: https://code.edge.launchpad.net/~maxb/meta-lp-deps/no-pullparser/+merge/16232 | 11:44 |
| adiroiban | jtv: if a person had not defined yet his/her preferred langauges, LP tries to be smart and provides a list with possible langauges | 12:17 |
| adiroiban | so we can not have an empty list of preferred languages | 12:18 |
| jtv | adiroiban: IIRC we've sort of been moving away from that, but I can't say I kept much of an eye on it | 12:18 |
| jtv | (we had endless fun with a unit test that used Serbia & Montenegro as a geoip example :-) | 12:19 |
| BjornT | maxb: approved | 12:21 |
| adiroiban | jtv: ok. then if there is no language I will set all launguages as preferred | 12:22 |
| maxb | BjornT: Thanks. | 12:22 |
| jtv | adiroiban: and don't forget to exclude en/en_US :-) | 12:22 |
| jtv | oh, just "en" I think | 12:22 |
| adiroiban | jtv: well I will just use the current list | 12:23 |
| adiroiban | jtv: i will finish it and push a branch for review | 12:23 |
| jtv | adiroiban: it all depends how many layers of abstraction you include, I suppose. :) In call now. | 12:23 |
| jtv | great! | 12:23 |
| === allenap changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: -, <lunch> || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === Ursinha-afk is now known as Ursinha | ||
| === jtv is now known as jtv-eat | ||
| === BjornT changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: -, <lunch> || queue [jpds, bjornt] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| BjornT | jtv-eat, allenap: i've added a small branch to the queue. it's not an rc candidate | 13:23 |
| * bac enjoying a quick breakfast | 13:38 | |
| bac | jpds: i'll have a look in a bit | 13:38 |
| === irc.freenode.net changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: -, <lunch> || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === allenap changed the topic of #launchpad-reviews to: on call: jtv, allenap || reviewing: -, <release-qa> || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === EdwinGrubbs changed the topic of #launchpad-reviews to: on call: jtv, allenap, Edwin || reviewing: -, <release-qa> || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === vednis is now known as mars | ||
| gmb | jtv, allenap: I have a branch for you to review: https://code.edge.launchpad.net/~gmb/launchpad/another-checkwatches-keyerror-bug-497414/+merge/16243 | 15:09 |
| gmb | Argh, netsplit. | 15:09 |
| === matsubara is now known as matsubara-lunch | ||
| gmb | jtv-eat allenap: I have a branch for you to review: https://code.edge.launchpad.net/~gmb/launchpad/another-checkwatches-keyerror-bug-497414/+merge/16243 | 15:12 |
| gmb | Sorry if you see this message twice; I got netsplit'd so I don't know who I hit and who I missed | 15:12 |
| allenap | jpds: Has someone picked up your branch? | 15:13 |
| === allenap changed the topic of #launchpad-reviews to: on call: jtv, allenap, Edwin || reviewing: -, - || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | gmb: How would all_remote_ids contain an id not in bug_watches_by_remote_bug? | 15:16 |
| jpds | allenap: bac's going to have a look at it. | 15:16 |
| gmb | allenap: Well, I haven't figured that out yet; that's a separate bug. But the point is that if we log the error rather than just blowing up when we hit it we can file a bug and investigate later rather than worrying about checkwatches failing all the time. | 15:17 |
| allenap | jpds: Cool. Thanks for being patient today :) | 15:17 |
| allenap | gmb: Oh, does it fail here too? | 15:18 |
| * allenap reads the bug report so as to stop wasting gmb's time :) | 15:20 | |
| allenap | gmb: Ah, so it does :) | 15:20 |
| gmb | allenap: Heh. | 15:21 |
| gmb | Yeah, the bug we just fixed was masking this one. | 15:22 |
| allenap | gmb: r=me | 15:26 |
| gmb | allenap: Awesome, thanks. | 15:26 |
| allenap | gmb: checkwatches is getting too complex to understand. | 15:27 |
| gmb | allenap: It did that years ago. It's since uploaded itself onto the web as pure energy and is responsible for all the major wars in the world. | 15:27 |
| allenap | gmb: Neuromancer, Wintermute... now CHECKWATCHES! | 15:28 |
| gmb | allenap: Doesn't *quite* have the same ring to it, but we can work on it. | 15:28 |
| === salgado is now known as salgado-lunch | ||
| EdwinGrubbs | allenap: can you review my branch? https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part1/+merge/16226 | 15:45 |
| allenap | EdwinGrubbs: Sure, I'll do it now. | 15:47 |
| === allenap changed the topic of #launchpad-reviews to: on call: jtv, allenap, Edwin || reviewing: -, Edwin, - || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adiroiban | henninge_: is there anything I need to do for https://code.edge.launchpad.net/~adiroiban/launchpad/bug-435165/+merge/16084 ? | 15:53 |
| EdwinGrubbs | henninge_: do you want me to review your branch? | 15:55 |
| adiroiban | there is also this on https://code.edge.launchpad.net/~adiroiban/launchpad/bug-435165/+merge/16084 :) | 15:55 |
| adiroiban | sorry. stupid paste | 15:58 |
| === henninge_ is now known as henninge | ||
| henninge | EdwinGrubbs: That would be really nice. | 16:05 |
| === jamalta is now known as jamalta|work | ||
| === matsubara-lunch is now known as matsubara | ||
| henninge | EdwinGrubbs: That would be really nice. I see you already claimed it. | 16:20 |
| EdwinGrubbs | yes, I'm working on it | 16:20 |
| === Ursinha is now known as Ursinha-lunch | ||
| leonardr | salgado-lunch: updated https://code.launchpad.net/~leonardr/launchpad/anonymous-oauth/+merge/16199 | 16:54 |
| === salgado-lunch is now known as salgado | ||
| salgado | leonardr, ok, will get to it in a minute | 16:59 |
| === henninge changed the topic of #launchpad-reviews to: on call: jtv, allenap, Edwin || reviewing: -, Edwin, henninge || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === abentley1 is now known as abentley | ||
| salgado | leonardr, did you reply to the review or just pushed a new version of the branch to launchpad? | 17:19 |
| === beuno-lunch is now known as beuno | ||
| leonardr | salgado: i replied with an incremental diff | 17:22 |
| leonardr | is it not there? | 17:22 |
| salgado | leonardr, nope. I think attachments are ignored | 17:23 |
| leonardr | salgado: i pasted a link to ubuntu pastebin | 17:24 |
| leonardr | 1 sec, on phone | 17:24 |
| === jtv-eat changed the topic of #launchpad-reviews to: on call: allenap, Edwin || reviewing: Edwin, henninge || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| EdwinGrubbs | henninge: I don't understand why there is both an IAccountModerate and an IAccountModerateStatus. Is that so that the status is viewable by anyone? | 17:31 |
| henninge | EdwinGrubbs: Yes, it is in IAccountPublic (and was before I touched it). | 17:33 |
| henninge | that is the reason for these two interfaces. | 17:34 |
| leonardr | salgado; doh! i never clicked 'save comment' | 17:34 |
| salgado | heh | 17:34 |
| leonardr | salgado: i found a bug in malone, but while i'm filing it... | 17:36 |
| leonardr | http://pastebin.ubuntu.com/342820/ | 17:36 |
| henninge | EdwinGrubbs: this whole interface-zcml relationship is quite confusing to me, too, and it took me a few attempts to get it right. | 17:37 |
| EdwinGrubbs | henninge: I'm wondering if it would be simpler to use <require set_attributes="status status_comment"> instead of <require set_schema="..."> which requires a complicated inheritance structure. | 17:39 |
| henninge | EdwinGrubbs: is there some kind of precedence of set_attributes over set_schema? | 17:40 |
| henninge | I mean, can an attribute be mentioned in an Interface and also explicitly in set_attribute? I think only that would help. | 17:41 |
| EdwinGrubbs | henninge: I've usually seen set_schema used when there are tons and tons of attributes and you want to avoid duplicating them in the zcml. I think using <require interface="..."> for viewing attributes but using set_attributes when set_schema doesn't map to an existing interface might work. | 17:45 |
| === Ursinha-lunch is now known as Ursinha | ||
| henninge | EdwinGrubbs: ok, I am happy to try that and get this whole thing simplified. | 17:46 |
| EdwinGrubbs | henninge: I also encountered these errors in the tests: http://pastebin.ubuntu.com/342855/ | 17:47 |
| henninge | EdwinGrubbs: huh, I ran that test all the time. But maybe not after the final changes. | 17:52 |
| === mrevell is now known as mrevell-dinner | ||
| === deryck is now known as deryck[lunch] | ||
| === allenap changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: henninge || queue [jpds] || RC candidates take priority || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara_ is now known as matsubara | ||
| rockstar | EdwinGrubbs, can I stick another branch on your queue? | 18:22 |
| rockstar | (it's not an RC candidate) | 18:24 |
| leonardr | salgado: a quick incremental diff for the launchpadlib branch. i'd like your opinion on a test change | 18:25 |
| leonardr | https://code.edge.launchpad.net/~leonardr/launchpadlib/anonymous-access/+merge/16213 | 18:25 |
| EdwinGrubbs | rockstar: sure, I'll start on it after lunch. | 18:29 |
| rockstar | EdwinGrubbs, thanks. | 18:29 |
| EdwinGrubbs | henninge: I sent you an intermediate review with a few comments on the code besides what we discussed already. | 18:38 |
| henninge | EdwinGrubbs: thanks. I got the interfaces figured out. | 18:39 |
| === EdwinGrubbs is now known as Edwin-lunch | ||
| === deryck[lunch] is now known as deryck | ||
| leonardr | salgado: about this code | 19:40 |
| leonardr | assert self.consumer is not None, ( | 19:40 |
| leonardr | "Need a registered consumer key for authenticated requests.") | 19:40 |
| leonardr | if i put an assert there, i can't verify that the server rejects an authenticated request with an unknown consumer key | 19:40 |
| leonardr | it gets rejected before the request is made | 19:41 |
| salgado | oh, ok, I didn't realize that | 19:42 |
| salgado | that's testing code, so I guess it's ok to be permissive | 19:42 |
| leonardr | i think i can use your structure and it'll be more understandable, at the cost of 1 duplicate line of code | 19:44 |
| leonardr | yeah, it's more readable | 19:47 |
| === matsubara is now known as matsubara-afk | ||
| leonardr | salgado: any idea why i would get this test failure? | 20:04 |
| leonardr | http://pastebin.ubuntu.com/342959/ | 20:04 |
| leonardr | the second part is my rearrangement of the code in response to your most recent comment | 20:04 |
| leonardr | i'm running the same test on a fresh launchpad to see where the error came in | 20:05 |
| salgado | leonardr, looks like we have a time bomb in our test suite | 20:06 |
| leonardr | salgado: yes, i get the error in a fresh launchpad, so i'm going to be a coward and not deal w/it | 20:09 |
| salgado | leonardr, a fresh devel or db-devel branch? | 20:10 |
| leonardr | salgado: devel | 20:10 |
| salgado | leonardr, it might be fixed in db-devel | 20:11 |
| === salgado is now known as salgado-afk | ||
| leonardr | salgado: argh, i have to do a new release of launchpadlib before i can land this branch, because the old launchpadlib contains test failures when run against the new launchpad | 20:16 |
| leonardr | (because the old launchpadlib has a test that verifies you can't log in to the web service when providing no credentials) | 20:16 |
| leonardr | salgado: so if you have an opinion on my new launchpadlib test (http://pastebin.ubuntu.com/342874/), i'd like to hear it | 20:20 |
| salgado-afk | leonardr, I think that extra test won't hurt, so I think it's ok to leave it | 20:57 |
| leonardr | salgado: cool | 20:57 |
| === Edwin-lunch is now known as EdwinGrubbs | ||
| rockstar | EdwinGrubbs, hi. | 21:09 |
| EdwinGrubbs | rockstar: hi, I saw that Tim reviewed your branch | 21:10 |
| rockstar | EdwinGrubbs, ah, it was Tim, not you. Okay, maybe I need to chat with him. He apparently knows something I don't. | 21:10 |
| rockstar | EdwinGrubbs, I thought it was you, and so I was going to ask some questions. | 21:10 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!