| === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: wallyworld || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| gmb | wallyworld_: Who was your pre-implementation contact for your branch? I ask because whilst your solution seems sane I don't have enough domain knowledge to know whether or not there's a better way to do it. | 12:47 |
|---|---|---|
| wallyworld_ | gmb: which one? the codehosting error leakage? | 12:48 |
| gmb | wallyworld_: Yes, sorry. | 12:48 |
| wallyworld_ | gmb: i talked with tim, aaron, and michael hudson. michael has an issue about raising a second oops but i couldn't see another way to do it. but i'm really new to this section of code. i'm sure tim wouldn't mind if you wanted to run any concerns by him | 12:50 |
| gmb | wallyworld_: I just wanted to check who you'd talked with about it; I'm not familiar with the code either. If Tim et. al. have given you their blessing that's good enough for me. | 12:51 |
| wallyworld_ | gmb: ok. i'll double check with tim to ensure he is 100% happy because i really don't want to screw anything up | 12:52 |
| wallyworld_ | thanks for looking at the code | 12:53 |
| gmb | wallyworld_: Cool, works for me. Best to request a review from him on the merge proposal so that we can track it. | 12:53 |
| gmb | wallyworld_: No problem. | 12:53 |
| gmb | wallyworld_: r=me. I've added thumper as a reviewer, too. | 13:05 |
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: jcsackett || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: henninge || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| henninge | gmb: thanks for picking that up ;) | 13:10 |
| gmb | :) | 13:13 |
| === mrevell is now known as mrevell-lunch | ||
| gmb | henninge: r=me with a couple of nitpicks. | 13:27 |
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| henninge | gmb: thanks | 13:37 |
| gmb | np | 13:39 |
| henninge | gmb: oops, I left the changes in migrate_current_flag.py in there by accident. That was just meant for debugging something else. | 13:40 |
| === leonardr changed the topic of #launchpad-reviews to: On call: gmb, leonardr || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| gmb | henninge: Heh, okay. | 13:48 |
| === mrevell-lunch is now known as mrevell | ||
| === matsubara is now known as matsubara-lunch | ||
| bac | hi gmb can i have a review of a small branch? | 14:27 |
| gmb | bac: Sure | 14:29 |
| bac | gmb: https://code.edge.launchpad.net/~bac/launchpad/bug-674897/+merge/42238 | 14:29 |
| bac | thanks | 14:29 |
| gmb | bac: r=me | 14:39 |
| bac | thanks graham | 14:42 |
| jcsackett | anyone available to do a quick ui review on https://code.edge.launchpad.net/~jcsackett/launchpad/launchpad-ids-270310/+merge/42190? | 14:48 |
| salgado | jcsackett, can it be in about 1h, after my lunch? | 14:58 |
| jcsackett | salgado: sure, that sounds great. | 14:58 |
| jcsackett | salgado: when you get to it, screenshot is in Demo in the proposal write-up. | 14:58 |
| salgado | cool, will remember that | 14:59 |
| === salgado is now known as salgado-lunch | ||
| === matsubara-lunch is now known as matsubara | ||
| adeuring | gmb, leonardr: could you please review this MP: https://code.edge.launchpad.net/~adeuring/launchpad/bug-596944-model/+merge/42253 ? | 15:17 |
| leonardr | adeuring, sure | 15:18 |
| adeuring | thanks! | 15:18 |
| leonardr | adeuring: out of curiosity, do projects want to turn this off because our detection algorithm doesn't work for them, or because 'duplicate' bugs are helpful to them? | 15:22 |
| adeuring | leonardr: many kernel or X related bugs are harware related. And you have one symptom caused by drivers for different hardware | 15:22 |
| leonardr | ok | 15:23 |
| adeuring | leonardr: this lets people say "yes, that's my bug", when it fact isn't | 15:23 |
| === salgado-lunch is now known as salgado | ||
| leonardr | adeuring, r=me | 16:08 |
| adeuring | leonardr: thanks! | 16:08 |
| === benji is now known as benji-lunch | ||
| jcsackett | salgado: sinzui grabbed the ui review i needed, so you're off the hook. | 16:55 |
| sinzui | oh, sorry | 16:56 |
| sinzui | I am still getting my head together after a week off | 16:56 |
| salgado | jcsackett, yeah, I noticed that when I got back from lunch | 16:57 |
| jcsackett | salgado: cool. | 16:57 |
| === benji-lunch is now known as benjui | ||
| === benjui is now known as benji | ||
| === gary_poster_ is now known as gary_poster | ||
| === henninge_ changed the topic of #launchpad-reviews to: On call: gmb, leonardr || Reviewing: - || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === henninge_ changed the topic of #launchpad-reviews to: On call: leonardr || Reviewing: - || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === henninge_ is now known as henninge | ||
| henninge | Hi leonardr! | 19:38 |
| henninge | Would be great if you could review my branch, please. ;) | 19:38 |
| henninge | https://code.launchpad.net/~henninge/launchpad/db-devel-bug-611668-filtermethods-2/+merge/42293 | 19:39 |
| leonardr | henninge: sure, if i can do it in the next 20 minutes... | 19:39 |
| henninge | have a look. | 19:39 |
| henninge | leonardr: it's oversized but has a lot of repetetive changes in it. | 19:39 |
| leonardr | henninge: describe to me the new translation model you mention in the mp? | 19:39 |
| henninge | translations used to be flagged as "current" and/or "imported" and we had different sets for a project and its linked source package. | 19:40 |
| henninge | the basis for the new model is that translations are shared between a project and its linked Ubuntu package. | 19:41 |
| henninge | so the meaning for the two flags changed and they were renamed. | 19:41 |
| henninge | They are now "is_current_ubuntu" and "is_current_upstream" | 19:42 |
| henninge | which indicate if a particular translation is used by both the project and the Ubuntu packaged or if the two translations differ. | 19:43 |
| henninge | scratch that second half | 19:43 |
| henninge | ... or if it is just used on one side. | 19:43 |
| henninge | so a lot of the transition is to make code "side aware", that is they have a concept on "this side" and "the other side". | 19:44 |
| henninge | meaning "upstream" and "Ubuntu" or vice versa, depending on wether you look at the translations for a project or for an Ubuntu package. | 19:44 |
| henninge | leonardr: can you follow ... ? ;-) | 19:45 |
| leonardr | yeah, i think so | 19:46 |
| henninge | The concept of "imported" is gone. | 19:46 |
| henninge | "imported translation", I should say | 19:47 |
| henninge | and has been replaced by "translation on the other side" | 19:47 |
| leonardr | henninge: it looks like in pofile.py you have a database field name obtained from a utility. is that ok? is that field name hard-coded in different utilities? | 19:47 |
| leonardr | or, rather, not from a utility but from some other data model object | 19:47 |
| henninge | the SideTraits | 19:47 |
| leonardr | is it hard-coded in different subclasses of SideTraits? | 19:48 |
| henninge | exactly | 19:48 |
| henninge | there are only two subclasses | 19:48 |
| henninge | "upstream" and "ubuntu" | 19:48 |
| leonardr | ok | 19:48 |
| henninge | and the flag names "is_current_ubuntu" and "is_current_upstream" are hardcoded in those classes. | 19:49 |
| henninge | (hence no quoting, if that is what you were wondering ;) | 19:49 |
| leonardr | henninge: why did makeTranslationMessage become makeCurrentTranslationMessage? just to avoid ambiguity? | 19:50 |
| leonardr | i saw that happened in the earlier branch too | 19:50 |
| leonardr | when do you pass in diverged=True and when not? what's going on in the different methods starting at line 212? | 19:51 |
| leonardr | (sorry for not digging in myself, but it'll go faster if you just answer my stupid questions) | 19:51 |
| henninge | that's ok | 19:51 |
| henninge | part of the change in this feature is that we got rid of an old method called "updateTranslation" which had grown out of proportion and is deeply rooted in the old model. | 19:52 |
| henninge | it has been replaced by a couple of light-weight methods. | 19:52 |
| henninge | makeCurrentTranslationMessage uses those new method, whereas makeTranslationMessage uses updateTranslation to create translation messages. | 19:53 |
| leonardr | henninge: what's the difference between a diverged translation and a changed translation? | 19:53 |
| henninge | the difference is between "diverged" and "shared" | 19:54 |
| henninge | "shared" translations are used across productseries or distroseries respectively. | 19:54 |
| henninge | "diverged" translations are pinned to a specific series and take precedence over shared ones. | 19:54 |
| * henninge looks at line 212 | 19:55 | |
| leonardr | henninge: i still don't understand what a 'changed' translation is | 19:56 |
| henninge | ah! | 19:56 |
| henninge | Changed is really an old term | 19:56 |
| leonardr | how come you usually change makeTranslationMessage to makeCurrentTranslationMessage, but on line 758 you change it to makeSuggestion? Is a suggestion a type of translation message, like a user-contributed one? | 19:57 |
| henninge | leonardr: exactly. It has no "is_current" flag set. | 19:58 |
| henninge | unprivileged uses can enter suggestions which can then be accepted by translation reviewers. | 19:58 |
| leonardr | henninge: StatistcsFiltersTestScenario should be Statistics... | 19:59 |
| henninge | which results in setting the is_current* flag | 19:59 |
| henninge | oh, right | 19:59 |
| leonardr | is this a class you intend to fill out? it doesn't do anything right now | 19:59 |
| henninge | StatistcsFiltersTestScenario? | 19:59 |
| henninge | it does. | 19:59 |
| henninge | It is used by TestUpstreamFilters and TestUbuntuFilters to make a test case. | 20:00 |
| henninge | They are all based on StatisticsTestScenario which has the actual tests. | 20:00 |
| henninge | the tests are all run from both sides "Upstream" and "Ubuntu" | 20:01 |
| henninge | leonardr: is that what you meant? | 20:01 |
| henninge | I still meant to explain about "changed" | 20:01 |
| leonardr | ok, i see what you mean. yes, explain about changed | 20:02 |
| henninge | It used to be that a translation was "imported" from a file, usually what the project had published upstream. | 20:02 |
| henninge | so it was imported with both "is_current" and "is_imported" set. | 20:03 |
| henninge | Now, if somebody went and changed that translation, effectively adding a new translation, the flags would be split between the two. | 20:03 |
| henninge | One is actually used, so "is_current", the other still shows how it was imported, so "is_imported" | 20:04 |
| henninge | The "ChangedInUbuntu" filter showed all strings for which a translation different from the imported one exists. | 20:04 |
| henninge | Since the "imported" concept has been replaced by the "upstream/ubuntu" concept, the Filter now shows where translations differ between the project and its linked Ubuntu package. | 20:06 |
| henninge | that's why I changed the name to "DifferentTranslations" | 20:06 |
| henninge | leonardr: is that clearer now? | 20:06 |
| leonardr | henninge: i believe i was confused by the code starting on line 624, where you use 'changed' in the sample data | 20:07 |
| henninge | I did? | 20:08 |
| leonardr | yeah, line 627, "changed translation" | 20:08 |
| leonardr | i don't really understand why it was "changed" and not "diverged", except that you didn't specify diverged=true | 20:09 |
| henninge | leonardr: look at the previous all to makeCurrentTranslation | 20:10 |
| henninge | line 615 | 20:10 |
| henninge | it specifies "current_other=True" | 20:10 |
| henninge | which means the translations is created with both flags set. | 20:10 |
| leonardr | is_current and is_imported | 20:10 |
| leonardr | ok | 20:10 |
| leonardr | if you set diverged=True then is_current is false and is_imported is true? | 20:11 |
| henninge | well, is_current_ubuntu and is_current_upstream nowaday | 20:11 |
| leonardr | and if you set nothing, what happens? | 20:11 |
| henninge | no flags == suggestion | 20:11 |
| henninge | "diverged" is not indicated by the flags but by another propety of the translation that links this translation to a specific translation template which in turn is linked to a specific series | 20:13 |
| henninge | if that property ("potemplate" btw) is None, the message is "shared". | 20:13 |
| leonardr | henninge: i absolutely must go, sorry. i can take another look when i come back (probably in about 2 hours), or you can find someone else, or we can take it up tomorrow | 20:14 |
| leonardr | i think i'm pretty close to approving this | 20:14 |
| henninge | I may or may not be around in two hours. | 20:14 |
| leonardr | ok, i'll ping you | 20:14 |
| henninge | thanks for taking your time to think into this | 20:15 |
| henninge | yes, please try. | 20:15 |
| === leonardr changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-afk | ||
| === salgado is now known as salgado-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!