launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

8	+ ordering = Int(
9	+ title=_("Parent ordering"), required=False,
10	+ default=1,
11	+ description=_(
12	+ "The parent ordering. Parents are ordered in "
13	+ "ascending order starting from 1."))

This might be more accurate as "Parent build dependency ordering. Parents are ordered in decreasing order of preference starting from 1"

117	+ self.parents = sorted(
118	+     parents_bulk,
119	+     key=lambda parent: self.parent_ids.index(parent.id))

You could use operator.itemgetter here instead of the lambda, I think it will be more readable.

199	+ dsp_set = getUtility(IDistroSeriesParentSet)
200	+ if self.overlays and self.overlays[0]:
201	+ pocket = PackagePublishingPocket.__metaclass__.getTermByToken(
202	+ PackagePublishingPocket, self.overlay_pockets[0]).value
203	+ component_set = getUtility(IComponentSet)
204	+ component = component_set[self.overlay_components[0]]

Presumably you want to stop hard-coding overlays[0] here :)

213	+ count = count + 1

count += 1

239	+ # XXX?? bool_and(official) ??

I'm not sure why you're asking about this, I think we can just copy that field from the parent DAS.  We don't need to add selection criteria on it.

255	+ # Take nominatedarchindep from the first parent. XXX??
256	self.distroseries.nominatedarchindep = self.distroseries[
257	- self.parent.nominatedarchindep.architecturetag]
258	+ self.parents[0].nominatedarchindep.architecturetag]

It's *probably* OK to do that, although we could put a warning on +initseries and they can change it later if desired.

446	+ # XXX?? What about 'related_set' in this case?
447	+ # (We are in the case where the packageset has already
448	+ # been created from another parent)

I think again we could do a similar thing and use the first parent as the choice for anything that clashes.  It will be hard to do anything else without more thought and we don't really have time right now.  It may come up as a bug in the future but that's fine.

I think the tests you've added so far are good although test_multiple_parents_initialize could do with checking that multiple packages from all the parents are in the new series.

Here's some ideas for extra ones:

 * the case where the same packageset is in two parents
 * the case where two parents have the same package (it should work if they are different versions)
 * the case where two parents have exactly the same package version (the copy will fail on the second package, we need to make sure it copes with this and ignores the second)
 * test cases where you're merging distinct components etc.

-- 
https://code.launchpad.net/~rvb/launchpad/init-series-bug-789091-devel/+merge/63676
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/init-series-bug-789091-devel into lp:launchpad.