Code review comment for ~pappacena/launchpad:update-stats-on-translation-copy

Revision history for this message
Colin Watson (cjwatson) wrote :

Hmm. Interesting, but I'm a bit confused. (Some of this confusion is because I don't know Translations very well.)

TranslationImporter.importFile is I think only called from POFile.importFromQueue and POTemplate.importFromQueue. I was going to say that I thought this sort of thing probably ought to go there rather than in the utility. But then I looked in more detail and found that the substance of it is supposed to be there already: if you look down towards the bottom of POFile.importFromQueue, it calls self.updateStatistics(), and if you look down towards the bottom of POTemplate.importFromQueue, it does a bunch of statistics updates as well. These are mostly similar to what POFileStatsJob does, so I think scheduling a POFileStatsJob here is mainly redundant and we shouldn't do it.

Of course, something is clearly wrong, but I'd approach it differently. Is it possible that the shared template handling in POFileStatsJob isn't done by the existing statistics update code in the models? I think that's probably what the problem is: we're already updating statistics for the POFile itself just fine (or for the POTemplate and all its POFiles), but we aren't dealing with other POFiles that share translations.

However, as I said, LP Translations really isn't my strong point and in particular I've never really got my head around translations sharing, so I'd suggest trying to get hold of William and discuss this with him.

review: Needs Information

« Back to merge proposal