launchpad-reviewers team mailing list archive

[Ursula Junque <ursinha@xxxxxxxxxxx>]

> 108     + def _findSourcePublication(self, packageupload):
> 109     + """Find destination source publishing record of the
> packageupload."""
> 110     + if packageupload.package_name is None:
> 111     + self.setComponents(self.tarfile_path)
> 
> It's a bit weird to have this take packageupload as an argument, but then use
> self.tarfile_path in the other path. I'd take filename as an argument.
> 
> 112     + return packageupload.archive.getPublishedSources(
> 113     + name=packageupload.package_name, exact_match=True,
> 114     + distroseries=packageupload.distroseries,
> 115     + pocket=packageupload.pocket).first()
> 
> That needs to use self.package_name if packageupload.package_name is unset.

I've changed process() to set self.package_name in the beginning so I always use it instead of setting it in _findSourcePublication. I don't like setComponents and parsePath, as the names aren't accurate for this case, but I'm using them as they have to be declared and have similar purpose.

> 
> 613     base_json_data=simplejson.dumps(
> 614     - {'sourcepackagerelease': sourcepackagerelease.id,
> 615     + {'packageupload': packageupload.id,
> 616     + 'sourcepackagerelease': sourcepackagerelease.id,
> 617     'libraryfilealias': libraryfilealias.id}))
> 
> The job actually only uses the sourcepackagename and distroseries. It'd be
> simpler, clearer and easier to test if this code didn't touch the
> sourcepackagerelease or packageupload.

Okay, I've removed both from the job code, now create() takes a distroseries, a libraryfilealias, a sourcepackagename and a boolean that says if it has shared templates. All other tests haven't changed much.

I also added a test to check if a package with translations, when uploaded to a PPA then copied to the main archive, has its translations processed and imported.

> 
> Your test changes look good. But we'll want to QA this fairly heavily on
> dogfood, and not deploy it until saucy's well released.
-- 
https://code.launchpad.net/~ursinha/launchpad/qa-bad-bug-1201485-rebased/+merge/191077
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.