Code review comment for lp:~rvb/launchpad/init-multiple-parents-packagesetpicker

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Thanks for making these changes. Much better now!

One thing that still needs fixing, as mentioned on IRC: in the test, the two distroseries have the same id. That happens to produce the right results here, but it does upset the bookkeeping a little bit.

The ordering of the ids in the test comparison seems a little arbitrary (5, 7, 6, 4 IIRC). If possible, use ordering-insensitive comparisons where ordering is not important and test ordering separately. Less brittle, easier to comprehend.

But now I'm nit-picking. Well done!

Jeroen

review: Approve (js)

« Back to merge proposal