[10:41] <jtv> No volunteers for a review today?
[12:15] <henninge> jtv: Let's swap: https://code.edge.launchpad.net/~henninge/launchpad/bug-516736-templatename/+merge/23659
[12:16] <jtv> henninge: OK!
[12:20] <jtv> henninge: s/\<otemplate/potemplate/
[12:24] <jtv> henninge: also, it may help to introduce a variable in the last paragraph of the diff:
[12:24] <jtv> potemplate = self.context.potemplate
[12:24] <jtv> if potemplate is None:
[12:24] <jtv>     # ...
[12:24] <jtv> else:
[12:24] <jtv>     # Do stuff with potemplate, not self.context.potemplate
[12:25] <henninge> jtv: yeah, nice idea. Although one line lost, one line gained...
[12:32] <henninge> jtv: how did you "tighten the check a bit more" ?
[12:34] <jtv> henninge: besides just counting the number of forms, I'm also checking that they are the right forms.  The old code would not detect the problem with nplurals=2 and "n ? 0 : 2" for instance.
[12:34] <henninge> oic
[12:34] <jtv> henninge: anyway, I'm approving yours.
[12:34] <henninge> jtv: is it this?
[12:35] <henninge> 144	- if pluralforms_count != len(forms):
[12:35] <henninge> 145	+ found_forms = sorted(forms.keys())
[12:35] <henninge> 146	+ if found_forms != range(expected_forms):
[12:35] <jtv> henninge: right
[12:35] <henninge> cool
[12:35] <henninge> jtv: thanks
[12:35] <jtv> It may seem wasteful at first to do a range comparison, but this is only over the number of forms anyway—maximum of 6
[12:36] <henninge> jtv: I just have two small things, I'll mention them in the review. r=me
[12:37] <jtv> henninge: dankeschöen
[12:37] <jtv> ahem
[12:37] <henninge> doppelt gemoppelt
[12:37] <jtv> ön
[12:37] <henninge> ;-)
[12:44] <henninge> jtv: review sent
[12:44] <jtv> thanks
[15:28] <sinzui> BjornT, ping
[15:29] <BjornT> hi sinzui
[15:30] <sinzui> I am not certain from your remarks about the not-packaged-1 branch whether I should try a different name for a new column, or proceed using Packaging
[15:32] <BjornT> sinzui: it depends on your use case. if you actually want to record that something isn't packaged, and display this information in the ui, go for the Packaging table.
[15:33] <sinzui> BjornT, regarding the enum, we hide the INCLUDE enum a few months ago because project owner do not understand it. It is used for some Debian and Ubuntu packages. While it is correct for modeling, it does not provide a lot of value for the project or ubuntu contributors
[15:33] <BjornT> sinzui: if all you want to do is to control whether to ask the user whether it's packaged, go for the column with a different name
[15:33] <sinzui> I will choose a different name since this is really about a portlet behaviour
[15:34] <BjornT> sinzui: ok, cool
[16:46] <adiroiban> danilos: Hi, when you have some time can you please take a look at the current interface proposed for bug 512735 and have a short preimplementation chat?
[16:46] <mup> Bug #512735: At first diplay only preferred languages for a POFile from a POFileSubset <tech-debt> <ui> <Launchpad Translations:In Progress by adiroiban> <https://launchpad.net/bugs/512735>
[17:03] <danilos> adiroiban, sure
[17:03] <danilos> adiroiban, should I look at the branch?
[17:05] <adiroiban> yes
[17:05] <adiroiban> danilos: there is an IHasLangaugesWithTranslations interface
[17:06] <danilos> adiroiban, right, looking
[17:06] <adiroiban> and an implementation of language-statistics view
[17:06] <danilos> adiroiban, so, I am not sure this is quite enough; this is definitely a step in right direction though
[17:06] <adiroiban> danilos: what is not enough? the fields from the interface?
[17:07] <danilos> adiroiban, yeah, I am not sure that's completely enough
[17:10] <danilos> adiroiban, still looking though :)
[17:10] <danilos> adiroiban, the biggest concern is performance
[17:11] <danilos> adiroiban, also, I suspect you based the generic languages-translations-statistics.pt on one of distroseries or productseries templates
[17:11] <danilos> adiroiban, it would have been much easier to read that if you moved one of them to it
[17:12] <danilos> adiroiban, also, has_translations and languages_statistics should probably be @properties
[17:19] <adiroiban> danilos: true. my bad for not moving one of those files. And yes, those should be properties
[17:22] <adiroiban> danilos: the idea is that there will a lot of changes, so I should start by implementing an interface for distroseries and productseries and commit that one
[17:23] <adiroiban> and then see what other fields are required by sourcepakage and potemplate and add them later
[17:24] <danilos> adiroiban, sure, sounds good, that's why I am complaining about the templates, because that'd be the easiest way for me to see if this is indeed enough for the first go :)
[17:25] <danilos> adiroiban, btw, LanguageStatisticsView should definitely not be in browser/language.py :)
[17:26] <adiroiban> danilos: agree, I was not sure where to put it. Should I add it in a new file?
[17:26] <danilos> adiroiban, sure, please do... it's also not really "language_statistics", it's "per-language statistics"
[17:29] <danilos> adiroiban, so, I think it looks good in general, I don't like the naming for many things though :)
[17:29] <adiroiban> danilos: why do you think there will be performance issues?
[17:30] <danilos> adiroiban, well, we have to be careful about how many DB queries are executed and how many Storm objects we instantiate
[17:33] <adiroiban> is there an easy way of profiling sql query numbers and created storm objects ?
[17:33] <danilos> adiroiban, so, if we don't have everything we need exposed in the interface and we have to do something like go through all languages to get a total of something, we are a bit screwed (i.e. timeouts will happen)
[17:34] <adiroiban> yes, but those statistics are already generated
[17:34] <danilos> adiroiban, eg. imagine we need to get a total of translated and untranslated messages here; with this interface, we'd have to go through all languages and add them up
[17:34] <adiroiban> is just that there is a lot of duplicate code
[17:34] <danilos> adiroiban, right, I am just giving an example of what kind of things to watch out for
[17:35] <danilos> adiroiban, and that's the kind of thing we'd need to watch for when designing the interface
[17:35] <danilos> adiroiban, I am not pretending to be too smart this late in the day, though, so take that into consideration :)
[17:39] <danilos> adiroiban, anyway, if we can't think of anything else, I mostly have complaints about the naming :)
[17:39] <adiroiban> :)
[17:39] <danilos> adiroiban, not that I can come up with good names right now
[17:40] <adiroiban> I will try to rename then and will come back with a full implementation for distro and product series
[17:41] <danilos> adiroiban, sure, sounds good
[17:42] <danilos> adiroiban, the important distinction is that these are not languages, as I said above, but instead translations... perhaps using "translations" for the property name on the interface would be better
[17:44] <adiroiban> danilos: true... i was thinking language_translations
[17:44] <adiroiban> to denote that those translations are per language
[17:45] <danilos> adiroiban, right, I am struggling with coming up with good naming for this :)
[22:24]  * jpds has https://code.launchpad.net/~jpds/launchpad/fix_565345-mirror-prober-agent/+merge/23708 up for review.