launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

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

Done.

> 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.

Hum ... I had the same idea but I'm not sure it's possible to use operator.itemgetter here because what is done here is much complex that simply getting an item.

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

Good point :)

> count += 1

Okay ;) ... (I miss count++)

> 
> 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.

I figured many parents could have different 'official' status for the same (processorfamily, architecturetag) ... hence the question ... and my proposition was to try to come up with something more sensible than copying the value from one of the parents at random.

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

Ok.

> 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.

Ok.

> 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.

Yeah, I would need to refactor assertDistroSeriesInitialisedCorrectly to do that properly.

> Here's some ideas for extra ones:
> 
>  * the case where the same packageset is in two parents

That's the purpose of test_multiple_parent_packagesets_merge.

>  * 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.

Thanks for these ideas!
-- 
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.